-
Notifications
You must be signed in to change notification settings - Fork 334
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
refactor!: unify FrontendOptions
and DatanodeOptions
by using GrpcOptions
#4088
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4088 +/- ##
==========================================
- Coverage 84.92% 84.85% -0.07%
==========================================
Files 1023 1020 -3
Lines 179298 179296 -2
==========================================
- Hits 152272 152146 -126
- Misses 27026 27150 +124 |
Most of the changes are look good to me, except:
|
This comment was marked as outdated.
This comment was marked as outdated.
FrontendOptions
and DatanodeOptions
by using GrpcOptions
FrontendOptions
and DatanodeOptions
by using GrpcOptions
Since many people may already set the old |
Yes, I'm aware of this, but still exist some other questions for fuzz tests within distributed mode. I'll ping again when everything is ready. |
I guess some of the fuzz tests use the old rpc options. |
test: add test_depreacted_cli_options unit test
Hi, all, please take a look when you have some free time.
This PR moves
It has been resolved. I guess the distributed fuzz tests failed previously because they were not compatible with the greptimedb cluster's configuration used by CI. |
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.
Looks good.
Co-authored-by: Yingwen <[email protected]>
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.
Great work! Thanks a lot.
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
As mentioned in the comment at #3957 (comment), try to unify
FrontendOptions
andDatanodeOptions
by usingGrpcOptions
.What's changed and what's your intention?
GrpcOptions
to servers/src/grpcDatanodeOptions
rpc_addr
by Option. e.g. rpc_addr: Option, set them to None by default and print a warning if people set it(refactor!: unifyFrontendOptions
andDatanodeOptions
by usingGrpcOptions
#4088 (comment)). The final result is as follows:Checklist