Migrate 6 API versions to shared arm_ml_service client - #47787
Merged
saanikaguptamicrosoft merged 149 commits intoJul 27, 2026
Merged
Migrate 6 API versions to shared arm_ml_service client#47787saanikaguptamicrosoft merged 149 commits into
saanikaguptamicrosoft merged 149 commits into
Conversation
…ression DatastoreOperations.create_or_update was migrated to the arm_ml_service client (which serializes the request body via json.dumps(body, cls=SdkJSONEncoder)) in PR Azure#47554, but the datastore ENTITIES still returned v2023_04_01_preview msrest Datastore models. SdkJSONEncoder can only serialize arm hybrid models, so every datastore create/update raised: TypeError: Object of type Datastore is not JSON serializable The bug was invisible to CI: unit tests mock the client (body never serialized), the datastore create e2e tests are skip/live_test_only, and notebook samples reuse the workspace default datastore instead of creating one. Fix: migrate the datastore entities (Blob/File/ADLS Gen2/Gen1/OneLake) and the shared datastore credentials to the arm_ml_service hybrid models so _to_rest_object() serializes cleanly through the operation client's encoder. Adds tests/datastore/unittests/test_datastore_serialization_regression.py: an offline Class-A (mixed-tree) guard that serializes each datastore body via SdkJSONEncoder exactly as the operation does. Note: the experimental HDFS on-prem datastore (model absent in arm_ml_service) is migrated separately via JSON-direct in the broader version migration.
saanikaguptamicrosoft
requested review from
JustinFirsching,
NonStatic2014,
achauhan-scc,
arunsu,
jayesh-tanna,
kingernupur,
nick863,
novaturient95,
rtanase,
sharma-riti and
vivram
as code owners
July 1, 2026 07:25
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a production serialization regression in azure-ai-ml datastores. In PR #47554, DatastoreOperations.create_or_update was migrated to the shared arm_ml_service generated client, whose operation serializes the request body via json.dumps(body, cls=SdkJSONEncoder, ...). However, the datastore entities still emitted v2023_04_01_preview msrest models, which SdkJSONEncoder cannot serialize, so every datastore create/update raised TypeError: Object of type Datastore is not JSON serializable. This PR migrates the datastore entities (Blob/File/ADLS Gen2/Gen1/OneLake) and the shared datastore credential conversions to the arm_ml_service hybrid models, and adds an offline regression test that serializes each datastore body exactly as the operation does.
Changes:
- Repoint datastore entity/credential/schema REST imports from
v2023_04_01_previewtoarm_ml_serviceso_to_rest_object()produces an all-hybrid tree that serializes cleanly. - Add
test_datastore_serialization_regression.py, an offline guard that runs each datastore body throughSdkJSONEncoder. - Update
test_datastore_schema.pyassertions to expectarm_ml_servicemodel types.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
entities/_credentials.py |
Move datastore credential/secret REST imports to arm_ml_service; surfaces a latent resource_uri vs resource_url mismatch for certificate creds (see comment). |
entities/_datastore/datastore.py |
Base datastore Datastore/DatastoreType imports moved to arm_ml_service. |
entities/_datastore/azure_storage.py |
Blob/File/ADLS Gen2 REST models moved to arm_ml_service. |
entities/_datastore/adls_gen1.py |
ADLS Gen1 REST models moved to arm_ml_service. |
entities/_datastore/one_lake.py |
OneLake REST models moved to arm_ml_service. |
_schema/_datastore/one_lake.py |
DatastoreType/OneLakeArtifactType enums moved to arm_ml_service. |
tests/datastore/unittests/test_datastore_serialization_regression.py |
New offline serialization regression guard (account-key & service-principal creds only). |
tests/datastore/unittests/test_datastore_schema.py |
Assertions updated to expect arm_ml_service model types. |
Additional notes (outside the PR diff, so not inline comments): the CHANGELOG.md "1.35.0 (unreleased) → Bugs Fixed" section is still empty; per the contribution checklist this bug fix should be documented there.
Addresses PR review: CertificateConfiguration._to_datastore_rest_object passed
resource_uri= to the arm_ml_service CertificateDatastoreCredentials model, but
that model's field is resource_url. The old msrest model silently ignored the
unknown resource_uri kwarg; the arm hybrid model raises
TypeError: CertificateDatastoreCredentials.__init__() got an unexpected
keyword argument 'resource_uri'
so every certificate-authenticated datastore (ADLS Gen1/Gen2) create/update
would crash - the same serialization-regression class this PR fixes.
Fix both the write (resource_url=self.resource_url) and read
(resource_url=obj.resource_url) paths, and extend the serialization regression
test to cover certificate credentials on ADLS Gen1 and Gen2 (the original test
only exercised account-key and service-principal creds).
Migrate the asset entities off the v2023_04_01_preview msrest models onto the shared arm_ml_service hybrid client: - Data / Model / Environment version models -> arm_ml_service. - AutoDeleteSetting, IntellectualProperty, ModelConfiguration are absent from arm_ml_service (2025-12); serialize them JSON-direct (plain camelCase dict) and read them back via mapping access, per the reuse-existing-client approach. - data.py: preserve the `autoDeleteSetting` wire field via mapping assignment (dropped from the arm model) and gate referenced_uris on the arm field set. - environment.py: read intellectual_property defensively (arm dict or msrest), coerce is_anonymous default to False (arm hybrid defaults to None). - component.py / pipeline_component.py: IntellectualProperty._to_rest_object now returns the wire dict directly, so drop the obsolete .serialize() call. Validated: model, environment, dataset, model_package, component unit suites (254 passed).
…024-01) Migrate the entire AutoML entity surface (tabular, image, and NLP jobs plus their settings/search-space models) off the per-version msrest restclients onto the shared arm_ml_service hybrid client, using the proven to_hybrid_rest_model boundary pattern for shared children (outputs/identity). - Tabular: regression/classification/forecasting jobs + featurization/limit/ training/forecasting settings, CustomTargetLags values->values_property, n_cross_validations/forecast-horizon discriminated children as wire dicts. - Image: 4 image jobs + model/sweep/limit settings via the boundary pattern. - NLP: text classification / multilabel / NER jobs; arm-absent NlpFixedParameters, NlpParameterSubspace, and NlpSweepSettings emitted JSON-direct; fixedParameters/ searchSpace/sweepSettings/maxNodes carried as wire-keys; local NlpLearningRate scheduler + TrainingMode enums replace the arm-absent generated enums. All 262 AutoML unit tests pass.
…axNodes wire fields
…/isAnonymous/conf-str wire
…m-wrap in spark envelope
…o fix mixed-tree serialize
… via as_attribute_dict
Flip the SweepJob envelope and its children to the shared arm_ml_service hybrid model, wrapping each nested child (trial distribution/resources, sampling_algorithm, limits, early_termination, objective, inputs, outputs, identity, queue_settings, resources) through to_hybrid_rest_model so the body serializes via the hybrid SdkJSONEncoder. Set is_archived=False and attach the arm-dropped `resources` field via wire-key after construction. - sampling_algorithm read path: arm hybrids are MutableMapping (not dict subclass), so read the arm-dropped `logbase` unknown wire key via _is_model/Mapping check instead of isinstance(obj, dict). - Read path _load_from_rest updated for arm mapping access (resources, searchSpace). - Tests: flip identity expected-models (AmlToken/ManagedIdentity/ UserIdentity) to arm_ml_service; introspect arm-dropped logbase/resources via mapping access; DSL test_set_limits expects camelCase jobLimitsType from arm CommandJobLimits as_dict. Sweep UTs (82) + sweep wire smoke (6) + full pipeline_job (200) + dsl + command + job_common all green. Retires v2023_08 consumers: sweep_job, objective, sampling_algorithm, SweepJobLimits.
…ml_service Flip the three sweep leaf models off v2023_08_01_preview onto the shared arm_ml_service hybrid models, now that the SweepJob envelope (and the arm-aware DSL node serializer) consume them. - objective.py: RestObjective -> arm. - sampling_algorithm.py: Random/Grid/Bayesian/base + SamplingAlgorithmType -> arm. arm RandomSamplingAlgorithm dropped `logbase`; preserve it as an unknown wire key on the hybrid model in _to_rest_object (set only when present) and read via _is_model/Mapping in _from_rest_object. - job_limits.py: SweepJobLimits RestSweepJobLimits -> arm (CommandJobLimits was already arm). Validated: sweep UTs (82) + sweep wire smoke + command_job UTs (108 total), full dsl + pipeline_job (490 passed, 2 skipped). Retires three more v2023_08 consumers.
…arm_ml_service Flip AzureOpenAiFineTuningJob and AzureOpenAiHyperparameters off v2024_01_01_preview onto the shared arm_ml_service hybrid models, mirroring the already-arm custom_model_finetuning_job. - azure_openai_finetuning_job.py: RestFineTuningJob/AzureOpenAiFineTuning -> arm; drop the msrest _resolve_inputs/_restore_inputs overrides (the arm FineTuningVertical base already provides arm-correct versions); rewrite _to_rest_object to build the arm envelope like custom_model. - azure_openai_hyperparameters.py: RestAzureOpenAiHyperParameters -> arm so the child serializes in the arm tree (fixes Class-A smoke crash where the msrest child couldn't serialize under the hybrid SdkJSONEncoder). - test_finetuning_job_convesion.py: repoint Aoai* isinstance assertions to arm types; properties bool coerces to str "True" per arm Dict[str,str] wire contract (aligns with existing custom_model test). Validated: finetuning UTs + full smoke_serialization (184 passed, incl byte-identical aoai wire oracle). Finetuning family now fully off v2024_01 (only distillation_job.py remains on that version).
Covers the certificate datastore credential (guards the cert credential wire; resource_url intentionally omitted since its migration fix is a latent bug-fix, documented in the builder) and the credential-less (identity-based) datastore path. Byte-identical vs baseline, no mocking.
…Ray distribution) Adds the arm-absent, hand-built JSON-direct families that are the highest regression risk (the same class as the distillation properties-bag bug): HdfsDatastore with Kerberos password creds, DataImport with Database and FileSystem sources, and a Ray-distribution CommandJob. Byte-identical vs baseline, no mocking.
Adds a Model with default/allowed deployment templates (arm-absent template fields with special wire-keying and the msrest stage-drop) and a WorkspaceAssetReference (arm-absent registry copy, hand-built JSON-direct wire). Byte-identical vs baseline, no mocking.
This comment has been minimized.
This comment has been minimized.
…ry-create None
Two notebook-caught regressions in the Environment migration:
1. inference_config passed as a raw snake_case dict (direct Environment(...) construction) was serialized verbatim by the arm hybrid encoder, so the server rejected it ('must specify all three of: liveness, readiness and scoring routes'). msrest's typed field mapped it snake->camel. Now convert the dict to an arm InferenceContainerProperties (livenessRoute/readinessRoute/scoringRoute); already-model inputs pass through unchanged.
2. Registry environment create wrapped the LRO result in EnvironmentVersion._deserialize eagerly; the registry create can return an empty (None) body, causing 'NoneType has no attribute items'. Only deserialize non-None; otherwise fall through to the existing _get re-fetch guard.
…e None guard
Regression coverage for the two notebook-caught Environment bugs:
- smoke: Environment(inference_config={raw dict}) and the arm-model path must both emit camelCase livenessRoute/readinessRoute/scoringRoute (explicit assertion; baseline cannot serialize the raw-dict input).
- unit: registry create returning an empty (None) LRO body must re-fetch via _get instead of crashing in _deserialize(None).
…vironment fix) Static audit of _deserialize(<registry create helper>) sites found the same latent bug as the environment registry create: code and data registry create wrapped begin_create_or_update_registry_versioned_asset in _deserialize eagerly, so an empty (None) LRO body would raise 'NoneType' object has no attribute 'items' before the existing 'if not result: re-fetch' guard could run. Only deserialize non-None results.
This comment has been minimized.
This comment has been minimized.
Contributor
[Pilot] PR Pipeline Failure AnalysisA CI pipeline failed on this pull request. Here is an automated analysis of what went wrong and how to get the build green. What failedThe Pylint check for
Recommended next steps
Raw pipeline analysis (azsdk ci analyze)
|
…nt docstring Fixes several AttributeError regressions where migrated read paths call msrest-only APIs on arm_ml_service hybrid models (which are MutableMappings without those attributes): - kubernetes_compute._load_from_rest: prop.additional_properties.get(createdOn) -> mapping-safe read (like aml_compute already does). - pipeline_component_batch_deployment._from_rest_object: deployment.properties.additional_properties[deploymentConfiguration] -> mapping-safe read. - _model_operations package(): additional_properties[targetEnvironmentId] else-branch made mapping-safe. - Registry create (code/data/environment): the create LRO body can be incomplete (missing the asset id, which _from_rest_object parses via AMLVersionedArmId -> 'no attribute asset_name'); re-fetch via get() when the id is missing. - environment: add pylint docstring param/return/rtype to _to_rest_inference_config (fixes Build Analyze C4739/C4741/C4742).
…O response) The registry create re-fetch guard (adf137a) re-fetches via get() when the create result lacks an id. test_show_progress_disabled_env mocked begin_create_or_update_registry_versioned_asset to return {properties:{}} (no id), which triggered a _get on the None mock registry client. The real registry create LRO returns the resource with its id; make the mock realistic so the guard is a no-op and the test exercises show_progress passthrough as intended.
achauhan-scc
approved these changes
Jul 27, 2026
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.
Notes
Testing
Description
Please add an informative description that covers that changes made by the pull request and link all relevant issues.
If an SDK is being regenerated based on a new API spec, a link to the pull request containing these API spec changes should be included above.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines