Skip to content

fix(sagemaker-core): drop tags before rebuilding TransformJob in tran… - #6110

Merged
lucasjia-aws merged 1 commit into
aws:masterfrom
lucasjia-aws:fix/6090
Jul 28, 2026
Merged

fix(sagemaker-core): drop tags before rebuilding TransformJob in tran…#6110
lucasjia-aws merged 1 commit into
aws:masterfrom
lucasjia-aws:fix/6090

Conversation

@lucasjia-aws

Copy link
Copy Markdown
Collaborator

…sform()

Transformer.transform() submitted the CreateTransformJob request successfully, then rebuilt a local TransformJob resource from the same request dict. That dict contains a "tags" key (Tags is a member of the CreateTransformJobRequest shape), but the TransformJob resource model has no tags field and sets extra="forbid", so the constructor raised a pydantic ValidationError after the job was already created on SageMaker, leaving the caller without a job handle.

Pop "tags" from the transformed dict before constructing TransformJob, mirroring the existing ProcessingJob fix (#5459). Add a regression test.

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…sform()

Transformer.transform() submitted the CreateTransformJob request
successfully, then rebuilt a local TransformJob resource from the same
request dict. That dict contains a "tags" key (Tags is a member of the
CreateTransformJobRequest shape), but the TransformJob resource model
has no tags field and sets extra="forbid", so the constructor raised a
pydantic ValidationError after the job was already created on SageMaker,
leaving the caller without a job handle.

Pop "tags" from the transformed dict before constructing TransformJob,
mirroring the existing ProcessingJob fix (aws#5459). Add a regression test.
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown

Claude finished @lucasjia-aws's task in 33s —— View job


PR Review: drop tags before rebuilding TransformJob

  • Read PR diff and understand the change
  • Verify fix against existing ProcessingJob pattern
  • Review test coverage

Verdict: Looks good — ship it.

The change is correct and minimal. Transformer.transform() was passing the full CreateTransformJobRequest dict (which includes tags) into TransformJob(**transformed), but the TransformJob resource model has no tags field and uses extra="forbid", so the constructor raised a ValidationError after the job was already created on SageMaker — leaving the caller without a job handle.

Popping tags before construction resolves this, and it faithfully mirrors the established fix in processing.py:637 (transformed.pop("tags", None)ProcessingJob(**transformed)), including the identical comment style. Using .pop("tags", None) is the right call — safe whether or not the key is present.

Observations (non-blocking):

  • Test quality: The regression test mocks TransformJob and asserts "tags" not in call_kwargs, which pins the exact behavior being fixed. Good. One caveat: because TransformJob is fully mocked, the test verifies the pop happens but does not exercise the real extra="forbid" validation path — so it wouldn't catch a future regression where the resource model starts accepting tags. That's an acceptable trade-off given the alternative (a full real-model construction) is heavier; just noting the test guards the fix, not the underlying model contract.
  • Consistency: Since this is now the second occurrence of the same tags-stripping workaround (processing + transform), it may be worth a follow-up to centralize it if a third job type needs it — but not something to block this PR on.

No correctness, security, or performance concerns. Nice tight fix with a targeted regression test.

@lucasjia-aws

Copy link
Copy Markdown
Collaborator Author

related to issue #6090

@lucasjia-aws
lucasjia-aws merged commit 317e291 into aws:master Jul 28, 2026
21 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants