Skip to content
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: Describe task config policies #9969

Merged
merged 14 commits into from
Oct 18, 2024
Merged

Conversation

tara-hpe
Copy link
Contributor

@tara-hpe tara-hpe commented Sep 23, 2024

Docs Created/Updated

Note: Feature is enterprise only

Description

Test Plan

Scenarios tested with screen captures:

priority_limit

2024-10-16_11-50-45

invariant_config

2024-10-16_11-50-45

complete example

2024-10-16_11-59-40

#example invariant config

2024-10-16_12-00-52

Checklist

  • Changes have been manually QA'd

@cla-bot cla-bot bot added the cla-signed label Sep 23, 2024
@tara-hpe tara-hpe marked this pull request as draft September 23, 2024 18:02
@determined-ci determined-ci requested a review from a team September 23, 2024 18:02
@determined-ci determined-ci added the documentation Improvements or additions to documentation label Sep 23, 2024
Copy link

netlify bot commented Sep 23, 2024

Deploy Preview for determined-ui canceled.

Name Link
🔨 Latest commit 9929dc8
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/6712bd327e39fd00082e4da9

@tara-hpe tara-hpe force-pushed the docs/describe-task-config-policies branch 3 times, most recently from 7b3915f to f9509b7 Compare September 25, 2024 16:57
@tara-hpe tara-hpe force-pushed the docs/describe-task-config-policies branch 5 times, most recently from 9a5ffe3 to ee028d8 Compare October 4, 2024 01:03
@tara-hpe tara-hpe marked this pull request as ready for review October 4, 2024 01:03
@tara-hpe tara-hpe force-pushed the docs/describe-task-config-policies branch from ee028d8 to ceaf527 Compare October 4, 2024 17:35
docs/manage/task-config-policies.rst Outdated Show resolved Hide resolved
docs/manage/task-config-policies.rst Outdated Show resolved Hide resolved
docs/manage/task-config-policies.rst Outdated Show resolved Hide resolved
@tara-hpe tara-hpe force-pushed the docs/describe-task-config-policies branch 3 times, most recently from 41fc612 to 9c93066 Compare October 9, 2024 21:23
@kkunapuli kkunapuli requested a review from johnkim-det October 10, 2024 17:55
@tara-hpe tara-hpe requested a review from kkunapuli October 11, 2024 14:59
docs/get-started/webui-qs.rst Outdated Show resolved Hide resolved
docs/manage/_index.rst Outdated Show resolved Hide resolved
docs/manage/task-config-policies.rst Outdated Show resolved Hide resolved
@tara-hpe tara-hpe force-pushed the docs/describe-task-config-policies branch 2 times, most recently from 31494c5 to 7093614 Compare October 11, 2024 20:07
@tara-hpe tara-hpe requested a review from johnkim-det October 11, 2024 20:09
Copy link
Contributor

@johnkim-det johnkim-det left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines 82 to 83
# Create a new policy
det policy create <policy_name> --config <policy_config_file.yaml>
Copy link
Contributor

@amandavialva01 amandavialva01 Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Create a new policy
det policy create <policy_name> --config <policy_config_file.yaml>
# Set config policies for a given scope.
det config-policies set <workload_type> --workspace <workspace_name> --config-file <policy_config_file>
where <workload_type> can be either `experiment` or `ntsc` and <policy_config_file> is the absolute or relative path the YAML or JSON file containing your config policies.
When creating global config policies, you don't need to specify `--workspace <workspace_name>`

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the note of the <workload_type> being experiment or ntsc I brought up using "NTSC" instead of "Tasks" in the webUI during a demo a couple of weeks ago, and someone (I think Tim?) mentioned that customers aren't very familiar with the NTSC terminology /categorization. If that's the case, it could be worth changing ntsc to tasks in the CLI for consistency (if time permits)!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's also important we use plural "config policies" everywhere!
We should probably mention somewhere in the docs that config policies are set together or not at all (updating config policies with the webUI or CLI will always update both your invariant config and constraints for a given scope, you cannot just update one of the two using our API)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amandavialva01 changes applied but, keeping this open in case you want to retain your note about the CLI. Do we need to share this with the CLI engineer?

Copy link
Contributor

@amandavialva01 amandavialva01 Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Saloni was the CLI developer, but she is currently on rotation on a different team! We currently have one ticket left for CLI work that I picked up.
But we have been tackling high priority fixes/work before making other changes. With that being said, I do think the only remaining "bug" fix is in review and a change like this could be made in the CLI ticket I linked (CM-576).
WDYT @kkunapuli ? Should we make this change after comments from the demo two weeks ago?

@tara-hpe tara-hpe force-pushed the docs/describe-task-config-policies branch from e151016 to 1e98075 Compare October 18, 2024 15:36
Copy link
Contributor

@amandavialva01 amandavialva01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome, Tara, fantastic work!!

docs/manage/config-policies.rst Outdated Show resolved Hide resolved
docs/manage/config-policies.rst Outdated Show resolved Hide resolved
docs/manage/users.rst Outdated Show resolved Hide resolved

.. note::

Tasks such as NTSC (notebooks, tensorboards, shells, and commands) and resource managers (RMs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Tasks such as NTSC (notebooks, tensorboards, shells, and commands) and resource managers (RMs)
Tasks such as NTSC (notebooks, tensorboards, shells, and commands) and experiments

docs/manage/config-policies.rst Outdated Show resolved Hide resolved
docs/manage/config-policies.rst Show resolved Hide resolved
docs/manage/config-policies.rst Outdated Show resolved Hide resolved
docs/manage/config-policies.rst Outdated Show resolved Hide resolved
docs/manage/config-policies.rst Outdated Show resolved Hide resolved
docs/manage/config-policies.rst Outdated Show resolved Hide resolved
@tara-hpe tara-hpe force-pushed the docs/describe-task-config-policies branch from d4ae6c3 to e14c106 Compare October 18, 2024 18:14
@tara-hpe tara-hpe force-pushed the docs/describe-task-config-policies branch 3 times, most recently from 8c8fd9e to 8361fa6 Compare October 18, 2024 19:47
@tara-hpe tara-hpe force-pushed the docs/describe-task-config-policies branch from 8361fa6 to 3063d7c Compare October 18, 2024 19:52
@tara-hpe tara-hpe merged commit b155332 into main Oct 18, 2024
76 of 90 checks passed
@tara-hpe tara-hpe deleted the docs/describe-task-config-policies branch October 18, 2024 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants