-
Notifications
You must be signed in to change notification settings - Fork 99
api: add numNodes spec #617
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
Conversation
d402d9f to
e7bde15
Compare
| // ComputeDomain. Must be zero or greater. Recommended to be set to zero: | ||
| // workload must implement and consult its own source of truth for the |
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 only recommend setting numNodes to 0 when IMEXDaemonsWithDNSNames=true. Setting it to 0 when IMEXDaemonsWithDNSNames=false will result in IMEX daemons being brought online without first filling out their nodes_config.cfg files with all indended IPs of the ComputeDomain. That was the whole point of the gating in the first place.
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.
only recommend setting numNodes to 0 when IMEXDaemonsWithDNSNames=true.
Of course. Will make an update.
This really is clear to me -- the fact that I forgot to make that case distinction explicitly is a reflection of how enthusiastic I am about leaving the "legacy world" behind us ASAP: precisely to not have to think through everything from two very different perspectives for a long period of time.
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.
Probably also worth saying that it must be equal to the expected number of workers joining the ComputeDomain when IMEXDaemonsWithDNSNames=false
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 have pushed another update. I like this discussion + iteration.
2e4971c to
5a0112c
Compare
Signed-off-by: Dr. Jan-Philip Gehrcke <[email protected]>
5a0112c to
d20a6a0
Compare
Add specification for current
numNodesbehavior as well as guidance towards non-reliance on the CD-globalStatusfield.As we remove this soon, it doesn't need to be perfect. But I'd like for us to get into the habit of having complete and correct specification of all parameters in our API surface; that will then be the reference for code -- and spec patches are some of the most interesting ones.
Resolves #615.