Skip to content

Feature: add user common types to pb impl#5289

Merged
bohutang merged 5 commits into
databendlabs:mainfrom
lichuang:from_serde_to_pb
May 10, 2022
Merged

Feature: add user common types to pb impl#5289
bohutang merged 5 commits into
databendlabs:mainfrom
lichuang:from_serde_to_pb

Conversation

@lichuang

@lichuang lichuang commented May 10, 2022

Copy link
Copy Markdown
Contributor

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

  1. add user common types to pb impl.
  2. add unit test cases of user pb impl.
  3. split old from_to_protobuf_impl.rs into {database|table|data}_rs.

Changelog

  • New Feature

Related Issues

Part of #5221

@vercel

vercel Bot commented May 10, 2022

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
databend ⬜️ Ignored (Inspect) May 10, 2022 at 3:47PM (UTC)

@mergify

mergify Bot commented May 10, 2022

Copy link
Copy Markdown
Contributor

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

@mergify mergify Bot added the pr-feature this PR introduces a new feature to the codebase label May 10, 2022

@drmingdrmer drmingdrmer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work man!

Comment on lines +30 to +32
pub use util::check_ver;
pub use util::missing;
pub use util::VER;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice:D

Comment on lines +24 to +40
message None {}
message Password {
enum PasswordHashMethod {
PlainText = 0;
DoubleSha1 = 1;
Sha256 = 2;
}
bytes hash_value = 1;
PasswordHashMethod hash_method = 2;
}
message JWT {}

oneof info {
None none = 1;
Password password = 2;
JWT jwt = 3;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What aboutt using a single Empty message for all kinds of enum variants that do not have a body?
Using Empty for None and JWT will get rid of several empty messages.

I.e., trade specificity for simplicity.

@bohutang
bohutang merged commit b3b8614 into databendlabs:main May 10, 2022
@bohutang

Copy link
Copy Markdown
Member

Add: Part of#5221 to PR summary.
Let's continue the work 🚀

@lichuang
lichuang deleted the from_serde_to_pb branch June 20, 2022 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-feature this PR introduces a new feature to the codebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants