-
Notifications
You must be signed in to change notification settings - Fork 800
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
Adding protoversion in config file #4263
Adding protoversion in config file #4263
Conversation
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.
I believe we only need the changes under docker/ folder. Config changes under config/ is not necessary as they will default to the right version.
@longquanzheng I'm not really familiar with cassandra but is there a case where someone is not using docker and wants to use a different protoversion? (either 2 or 3). if that's the case, wouldn't the config files the best way to do it? Just trying to understand this better! |
Good question. Yes It’s possible but these config files are meant to be
minimal for getting up and running for development. The full possible
configs are listed in our documentation.
https://cadenceworkflow.io/docs/operation-guide/setup/#static-configuration
That is a better place otherwise this config here will have to provide
every possible values. But it doesn’t have enough information to tell users
what they are.
also don’t worry about the document for your new config . The new release will have it automatically by godocs
On Fri, Jun 11, 2021 at 9:35 PM Rodrigo Villarreal ***@***.***> wrote:
@longquanzheng <https://github.com/longquanzheng> I'm not really familiar
with cassandra but is there a case where someone is not using docker and
wants to use a different protoversion? (either 2 or 3). if that's the case,
wouldn't the config files the best way to do it? Just trying to understand
this better!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4263 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABCQPM4WGGRAEZTCNRALP2LTSLP2FANCNFSM46MHBVLA>
.
--
Thanks,
Quanzheng
|
@longquanzheng friendly reminder to check this :) |
Pull Request Test Coverage Report for Build 913da920-7f17-43ba-93ba-061042fff29a
💛 - Coveralls |
What changed?
Continuation of #4252. ProtoVersion is now a config value
Why?
Labeled as
Up for grabs
asgood first issue
in #3536. PR was divided in 2 request, first one making sure compatibility was correct, second one adding the actual config valuesHow did you test it?
Playing with some values in the development.yaml file. Let me know what other ways I can test this
Potential risks
Connection to cassandra might be broken
Release notes
Maybe? Let me know if you want me to do that in the second step
Documentation Changes
Likely to be the case. Let me know if I need to do this in the second step