Skip to content

Document proto bytes field decoding with a test #1509

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 1, 2024
Merged

Document proto bytes field decoding with a test #1509

merged 1 commit into from
Feb 1, 2024

Conversation

Jorres
Copy link
Collaborator

@Jorres Jorres commented Feb 1, 2024

Changelog category

  • Not for changelog (changelog entry is not required)

Additional information

When invoking CopyFrom method on protobufs in Python, fields with a bytes type are automatically and implicitly base64-encoded. I've added a test to document that YDBD binary, when reading the config.yaml file, DOES NOT do the decoding automatically. In other words, in the business logic after reading the config, the base64-encoded string is used.

A real-world example to understand the case

This is related to cfg utility that is used to generate configs + proto txt files from a "cluster template yaml" file. When cfg is called with a template like this:

pdisk_key_config:
  keys:
  - container_path: "..."
    pin: "DqQEiklOLAFz/QmV/Pr4DQ=="
    id: "..."

The result in generated config.yaml looks like this, notice the DOUBLE base-64 encoding:

pdisk_key_config:
  keys:
  - {container_path: ..., id: ..., pin: RHFRRWlrbE9MQUZ6L1FtVi9QcjREUT09}

Copy link

github-actions bot commented Feb 1, 2024

2024-02-01 11:06:49 UTC Pre-commit check for 9af1611 has started.
2024-02-01 11:06:52 UTC Build linux-x86_64-release-asan is running...
🟢 2024-02-01 11:08:51 UTC Build successful.
2024-02-01 11:09:07 UTC Tests are running...
🔴 2024-02-01 12:41:56 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
16041 15906 0 33 57 45

Copy link

github-actions bot commented Feb 1, 2024

2024-02-01 11:06:50 UTC Pre-commit check for 9af1611 has started.
2024-02-01 11:06:52 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-02-01 11:08:52 UTC Build successful.
2024-02-01 11:09:07 UTC Tests are running...
🟢 2024-02-01 12:20:14 UTC Tests successful.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
60358 51055 0 0 9270 33

@Enjection Enjection merged commit f099e3d into ydb-platform:main Feb 1, 2024
@pavelvelikhov pavelvelikhov mentioned this pull request Feb 3, 2024
@niksaveliev niksaveliev mentioned this pull request Feb 5, 2024
@vitstn vitstn mentioned this pull request Feb 6, 2024
@vitstn vitstn mentioned this pull request Feb 7, 2024
This was referenced Feb 8, 2024
@starlinskiy starlinskiy mentioned this pull request Feb 12, 2024
@vitstn vitstn mentioned this pull request Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants