-
Notifications
You must be signed in to change notification settings - Fork 349
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
docs: add documentation for multirm #9016
Conversation
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
bab83eb
to
f75cbd2
Compare
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.
added comments w suggested edits
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.
see comments
- WebUI: Add ability to view resource manager name for resource pools. | ||
|
||
- API/CLI/WebUI: Route any requests to resource pools not defined in the master configuration to | ||
the default resource manager, not any additional resource manager, if defined. | ||
|
||
- Configuration: Add a ``name`` and ``metadata`` field to resource manager section in the master | ||
configuration. Add an ``additional_resource_managers`` section that follows the | ||
``resource_manager`` and ``resource_pool`` configuration pattern. |
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.
- WebUI: Add ability to view resource manager name for resource pools. | |
- API/CLI/WebUI: Route any requests to resource pools not defined in the master configuration to | |
the default resource manager, not any additional resource manager, if defined. | |
- Configuration: Add a ``name`` and ``metadata`` field to resource manager section in the master | |
configuration. Add an ``additional_resource_managers`` section that follows the | |
``resource_manager`` and ``resource_pool`` configuration pattern. |
These notes feel extraneous to me.
I think the release note should be pretty brief and just announce the feature and not get into the details.
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.
Where else would we put them? I think each of these changes are important for the end user to know -- we make release notes regularly for config changes, and changes in API behavior. We don't currently have a dedicated page to multiRM, so I thought they'd belong here
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 also like these updates in release notes for the reasons i posted in earlier comment. release notes are a great place to keep this info. if the users don't like it, they can skip it. it helps internally with cross referencing when we go back in later and check release notes against docs during doc reviews.
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.
added suggested edits and resolutions
419c6e3
to
cfa5313
Compare
This reverts commit 39afa3c.
This reverts commit 39afa3c.
Reverted
#9053
Description
Update configuration docs & add a release note for multirm.
Test Plan
N/A
Commentary (optional)
Checklist
docs/release-notes/
.See Release Note for details.
Ticket
RM-78