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: Clarify startuphook #9517

Merged
merged 3 commits into from
Jun 17, 2024
Merged

docs: Clarify startuphook #9517

merged 3 commits into from
Jun 17, 2024

Conversation

tara-hpe
Copy link
Contributor

@tara-hpe tara-hpe commented Jun 13, 2024

per customer request

startup-hook.sh does not apply to det cmd. It applies to experiments, notebooks, shells, and TensorBoards, but not commands.

Ticket

Description

Test Plan

Checklist

  • Changes have been manually QA'd
  • User-facing API changes need the "User-facing API Change" label.
  • Release notes should be added as a separate file under docs/release-notes/.
    See Release Note for details.
  • Licenses should be included for new code which was copied and/or modified from any external code.

@tara-hpe tara-hpe requested a review from channolanp June 13, 2024 18:13
@cla-bot cla-bot bot added the cla-signed label Jun 13, 2024
@determined-ci determined-ci requested a review from a team June 13, 2024 18:13
@determined-ci determined-ci added the documentation Improvements or additions to documentation label Jun 13, 2024
Copy link

netlify bot commented Jun 13, 2024

Deploy Preview for determined-ui canceled.

Name Link
🔨 Latest commit 1349496
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/667065229c01e20008f3cdbe

@tara-hpe tara-hpe force-pushed the docs/Clarify-startuphook branch from a45a0f5 to e869453 Compare June 13, 2024 18:15
are launched or deep learning operations are performed. The startup hook can be used to customize
the container environment, install additional dependencies, and download data sets among other shell
script commands.
If a ``startup-hook.sh`` file exists in the top level of your model definition directory, it is
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 "model definition directory" may be confusing. It is only really applicable to experiments. For shells, notebooks and tensorboards, it would, I think, just be called the context directory. It may be worthwhile saying you need to supply the context directory for the NTS(not C) commands, which should be --context, or -c for each of them. I'm sure you can get it to work with --include as well, but requires a bit more directory management.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@tara-hpe tara-hpe force-pushed the docs/Clarify-startuphook branch from e869453 to 929875c Compare June 17, 2024 15:05
per customer request
@tara-hpe tara-hpe force-pushed the docs/Clarify-startuphook branch from 929875c to db35e03 Compare June 17, 2024 15:05
@tara-hpe tara-hpe merged commit 1630c45 into main Jun 17, 2024
81 of 94 checks passed
@tara-hpe tara-hpe deleted the docs/Clarify-startuphook branch June 17, 2024 16:57
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.

3 participants