-
Notifications
You must be signed in to change notification settings - Fork 738
Fix bugs in Azure ML CI helper scripts (aml_creation.py, aml_attach_blob.py) - #190
Open
Manya Sharma (ManyaS-Git) wants to merge 1 commit into
Open
Fix bugs in Azure ML CI helper scripts (aml_creation.py, aml_attach_blob.py) #190Manya Sharma (ManyaS-Git) wants to merge 1 commit into
Manya Sharma (ManyaS-Git) wants to merge 1 commit into
Conversation
Fix several bugs in .ci/scripts/aml_creation.py and .ci/scripts/aml_attach_blob.py: - aml_creation.py: the workspace creation block was indented inside the option-parsing 'for' loop, so Workspace.create was invoked once per CLI option instead of once. Also removed leftover cookiecutter template placeholders after __main__. - aml_attach_blob.py: all five datastore CLI options (-dsn/-cn/-an/-ak/-drg) were assigned to 'workspace_region' due to a copy-paste error, leaving the actual variables unbound and raising NameError at Datastore registration. Each option now stores into its own variable. Added the missing Datastore import, fixed the same indentation bug, and corrected the usage message that incorrectly referenced aml_creation.py.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes several genuine bugs in the Azure ML CI helper scripts under
.ci/scripts/..ci/scripts/aml_attach_blob.py-dsn/--blob_datastore_name,-cn/--container_name,-an/--account_name,-ak/--account_key,-drg/--datastore_rg) were assigned toworkspace_region. The variables actually consumed byDatastore.register_azure_blob_container(...)were therefore never set, which would raise aNameErrorat runtime. Each option now stores into its own variable.Datastoreis used but was never imported fromazureml.core, which would also raiseNameError.for opt, arg in opts:loop, so the creation logic ran once per CLI option instead of once after parsing completed.aml_creation.pyinstead ofaml_attach_blob.py..ci/scripts/aml_creation.pyWorkspace.create(...)was indented inside the argument-parsingforloop and executed once per option.{{cookiecutter.*}}) that were never substituted.Verification
getoptparsing (same option tables) and confirmed each long option now assigns to the correct variable.tokenizethat the workspace/datastore creation block sits at function-body level (after the parsing loop) in both scripts.These scripts are referenced by the Azure DevOps templates (
creation_step.yml,deploy_steps.yml). No behavior change beyond fixing the bugs.