-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[ENH] Add proto conversions for RequestMetadata #2831
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
a2492bc
to
9655191
Compare
36b988e
to
de191d2
Compare
7d7efe6
to
2c6ff47
Compare
90caacb
to
52d7b8e
Compare
2c6ff47
to
d9fbe68
Compare
@@ -93,6 +93,11 @@ message OperationRecord { | |||
Operation operation = 4; | |||
} | |||
|
|||
message RequestVersionContext { | |||
uint32 collection_version = 1; | |||
uint64 log_position = 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: log_offset is the name used everywhere else. Don't have any strong preference on which is better but should we have a consistent naming everywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should. But this inconsistency is not introduced here (i.e "name used everywhere else" is not true").
If you look at message Collection {} for example
message Collection {
string id = 1;
string name = 2;
string configuration_json_str = 3;
optional UpdateMetadata metadata = 4;
optional int32 dimension = 5;
string tenant = 6;
string database = 7;
int64 log_position = 8;
int32 version = 9;
}
It is log_position. I went off what the collection called it, I don't know how to choose haha.
52d7b8e
to
cc48788
Compare
667bc3f
to
9bc5f5f
Compare
9bc5f5f
to
74f8e7f
Compare
Description of changes
Summarize the changes made by this PR.
Test plan
How are these changes tested?
pytest
for python,yarn test
for js,cargo test
for rustDocumentation Changes
None