install sm-spark-cli only if sagemaker workflows is enabled #915
Conversation
danobi06
requested review from
claytonparnell and
reganbaum
and removed request for
a team
|
|
||
| # Install sm-spark-cli | ||
| bash /etc/sagemaker-ui/workflows/sm-spark-cli-install.sh | ||
| bash /etc/sagemaker-ui/workflows/sm-spark-cli-install.sh || echo "Warning: sm-spark-cli installation failed, continuing..." |
There was a problem hiding this comment.
nit: can we put this print statement in the install script to keep the post startup script clean?
There was a problem hiding this comment.
Extending this one line is as clean as we can get to safe installing the script.
| # fallback to checking if only workflows blueprint exists | ||
| try: | ||
| blueprint_id = DZ_CLIENT.list_environment_blueprints( | ||
| managed=True, domainIdentifier=domain_id, name="Workflows" |
There was a problem hiding this comment.
IIRC users can modify blueprint names, and we've had issues in the past relying on the blueprint name. Can we get the environment blueprint by looking at the type or another field instead of the name?
There was a problem hiding this comment.
The workflows blueprint is a managed blueprint provided by sagemaker, customers won't be able to edit the blueprint. Unfortunately list environment blueprint only accepts searching by managed, domainIdentifier and name. ref
In the case users adds their own custom blueprint based off of workflows, then I agree this check won't suffice.
There was a problem hiding this comment.
Got it, thanks for clarifying. But the benefit is at least we don't hang anymore, so while not 100% fullproof, does still fix this edge case.
| # fallback to checking if only workflows blueprint exists | ||
| try: | ||
| blueprint_id = DZ_CLIENT.list_environment_blueprints( | ||
| managed=True, domainIdentifier=domain_id, name="Workflows" |
There was a problem hiding this comment.
Got it, thanks for clarifying. But the benefit is at least we don't hang anymore, so while not 100% fullproof, does still fix this edge case.
4 participants
Description
Add check to install spark cli only if one of the following conditions are true
Add
||logic to spark cli installation to gracefully fail and continue post startup execution.Type of Change
Release Information
Does this change need to be included in patch version releases? By default, any pull requests will only be added to the next SMD image minor version release once they are merged in template folder. Only critical bug fix or security update should be applied to new patch versions of existed image minor versions.
If yes, please explain why:
This change is needed to continue
sagemaker_ui_post_startup.shexecution in the eventsm-spark-clifails to install. In addition, sm-spark-cli will only be installed if sagemaker workflows blueprint exists.How Has This Been Tested?
Tested post startup script in an SMD environment by
bash /etc/sagemaker-ui/sagemaker_ui_post_startup.sh.Checklist:
Test Screenshots (if applicable):
Related Issues
[Link any related issues here]
Additional Notes
[Any additional information that might be helpful for reviewers]