-
Notifications
You must be signed in to change notification settings - Fork 15
rfc5: update broker module specification #489
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
|
|
| GitHub is NOT the preferred viewer for this file. Please visit | ||
| https://flux-framework.rtfd.io/projects/flux-rfc/en/latest/spec_5.html | ||
| 5/Flux Broker Modules |
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.
commit message "out of date" :-)
spec_5.rst
Outdated
| The colon-separated list of directories that SHOULD be searched for PATH | ||
| if it is not fully-qualified. |
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.
this section read funny to me, then I figured out what was confusing. Suggestion:
Do flux-module-exec MODULE
(aside, should you mention ARGS too?)
Then
The colon-separated list of directories that SHOULD be searched for MODULE if it not fully qualified
When I read "SHOULD be searched for PATH" for the environment variable "FLUX_MODULE_PATH", I was confused.
| The module name SHALL be registered as a default service provided by the | ||
| module. This means that request messages with topic strings that begin | ||
| with ``NAME.`` are routed to the module for handling. Additional services | ||
| MAY be dynamically registered by the module. |
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.
perhaps add an example, like kvs.lookup and kvs.commit are both routed to the kvs module?
|
|
||
| Module Attributes | ||
|
|
||
| Module Management |
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.
should there be some introductory text in this section? For each RPC?
mildly related, should the registering/unregistering of services in modules be here? Or for another RFC?
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 think elsewhere since it's not a requirement that the entity registering the service be a module.
|
Thanks, great suggestions. Sorry I think I kind of ran out of gas towards the end there. |
|
|
chu11
left a comment
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.
the changes LGTM!
Problem: RFC 5 is out of date. Make an update pass.
Merge Queue Status✅ The pull request has been merged at cfe43ed This pull request spent 5 seconds in the queue, with no time running CI. Required conditions to merge
|
|
|
Problem: RFC 5 is pretty of date at this point.
Make an update pass.