-
Notifications
You must be signed in to change notification settings - Fork 136
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
Hotfix import of AzureMLOnBehalfOfCredential to only credential-less datastore scenario. #3032
Conversation
assets/model_monitoring/components/src/model_monitor_create_manifest/run.py
Show resolved
Hide resolved
assets/model_monitoring/components/src/model_monitor_create_manifest/run.py
Show resolved
Hide resolved
assets/model_monitoring/components/src/shared_utilities/store_url.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have you tested with azure.ai uninstalled and the credential scenario works?
I have a test running where I manually removed azure-ai-ml from the pip dependencies of all components and ran the create manifest e2e test. It is not entirely comprehensive of the changes made but it covers a large percent of the changes. credential-less code-path Job w/o azure-ai-ml (expect failure): https://ml.azure.com/runs/good_bridge_yg74c7hr7d?wsid=/subscriptions/1aefdc5e-3a7c-4d71-a9f9-f5d3b03be19a/resourcegroups/momo-rampup-alanpo/workspaces/Project-alantest-oai&tid=72f988bf-86f1-41af-91ab-2d7cd011db47# credential code-path job w/o azure-ai-ml (expect success): https://ml.azure.com/runs/upbeat_farm_vb0b49b3n2?wsid=/subscriptions/5fca341e-4ec3-45bd-b006-a7648d9376c5/resourcegroups/modelmonitoring-e2e-rg/workspaces/momo-e2e-test-ws-eastus&tid=72f988bf-86f1-41af-91ab-2d7cd011db47# |
Hotfix for this PR here: #2929
We saw some issue with synapse cache using OLD cache dependency instead of NEW cache which included azure-ai-ml.
To remedy this need to move imports from new azure.ai.ml module to be inside of credential-less datastore code path to not impact old code path where we don't necessarily need to import from new conda module.
Note: for changing the try/catch from CredentialUnavailableError to an Exception here is an example job showing we still catch that error fine: https://ml.azure.com/runs/lime_branch_bz7qzy3b0s?wsid=/subscriptions/1aefdc5e-3a7c-4d71-a9f9-f5d3b03be19a/resourcegroups/momo-rampup-alanpo/workspaces/project-alantest-oai&tid=72f988bf-86f1-41af-91ab-2d7cd011db47#