Conversation
| aws s3 sync $WORKFLOW_PLUGINS_SOURCE_S3PATH $WORKFLOW_PLUGINS_PATH --delete | ||
| aws s3 cp $WORKFLOW_REQUIREMENTS_SOURCE_S3PATH $WORKFLOW_REQUIREMENTS_PATH | ||
| aws s3 cp $WORKFLOW_STARTUP_SOURCE_S3PATH $WORKFLOW_STARTUP_PATH |
There was a problem hiding this comment.
we should probably wrap these calls in a try/catch, and if it fails, then assume the FF isn't enabled as a fallback.
It's also probably useful for debugging purposes later on to include some echo statements throughout this logic so if we ever need to get the logs from the customer, we can see what happened more easily. Could we add in the try/catch and echo statements (e.g., print if the v2config is enabled, when it starts/finishes something, after receiving the blueprint id, etc)
There was a problem hiding this comment.
There is overall exception handling for this block here: https://github.com/aws/sagemaker-distribution/blob/main/template/v2/dirs/etc/sagemaker-ui/workflows/start-workflows-container.sh#L173
I don't know that falling back to the pre-FF behavior is the correct approach. Say this new logic has already successfully ran previously and now the s3 calls fail. Now you'll end up copying/using the old config files instead of the new ones and your local and shared environments will differ. I think the current logic of catching the failure and exiting immediately is more appropriate.
Will add additional logging though.
Description
Refactors workflows startup script to implement new decoupled setup flow. Logic branching determined by
enableWorkflowsConfigV2feature flag.If feature flag enabled:
.workflows_setupplugins/user-requirements.txtandplugins/user-startup.sh.Otherwise, existing logic is used.
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:
While not a bug fix/security update, feature flag will be enabled world-wide, so this change should be present in all active image versions.
How Has This Been Tested?
.workflows_setup(as feature flag not yet enabled) and containers started:a) config files pulled from s3 bucket during post-startup:
.workflows_setupand containers started:Checklist:
Test Screenshots (if applicable):
Related Issues
N/A
Additional Notes
--deleteflag). This was done to remove the old system plugins in existing spaces.