SEPURI-SAI-KRISHNA opened a new pull request, #76905:
URL: https://github.com/apache/airflow/pull/76905
`SageMakerRegisterModelVersionOperator` documents two separate descriptions:
`package_group_desc` for the model package group it may create, and
`package_desc` for the model package version it registers. It stores both,
templates both, and then passes `package_desc` to both:
```python
group_created =
self.hook.create_model_package_group(self.package_group_name, self.package_desc)
```
The hook's second parameter is `package_group_desc`, so the group is created
with the version's description. The operator's own value is then read nowhere:
before this change `self.package_group_desc` appeared exactly once in
`providers/amazon/src`, on the assignment line in `__init__`. A Dag that sets
it, including through a template, gets nothing.
The group is created only when it does not already exist, and
`create_model_package_group` returns `False` when it is there. So the
description is written once and then every later version registered into the
group inherits a group described by whichever version happened to create it.
This dates to #28024, which added the operator in 2022 with the call in this
form. #46590 moved the file during the provider restructure without touching
the line.
**Why the existing test did not catch it**
`test_execute` asserted
`create_group_mock.assert_called_once_with("group-name", "")` while setting
neither description, so both arguments were the same empty string and the swap
was invisible.
**The fix**
The two values are both optional strings in the same order, which is what
let them be swapped, so the call now names them.
`create_model_package_group(package_group_name=..., package_group_desc=...)`
cannot silently take the wrong one, and a future rename of either parameter
fails loudly instead of shifting a value.
**Behaviour change**
A group created from now on carries `package_group_desc` rather than
`package_desc`. Groups that already exist are untouched, since the operator
does not update them. Anyone who set only `package_desc` and read the result in
the SageMaker console will see new groups described as empty rather than
carrying the version text, which is the documented contract for both parameters.
**Tests**
`test_group_and_package_keep_their_own_description` sets the two
descriptions to different strings and asserts that the group gets its own and
that `ModelPackageDescription` still gets the version's. It is the only new
test, and it fails on the current code with the group receiving `trained on the
October data` instead of `every fraud detection model`.
It asserts on the boto call rather than on a mocked hook method, so it
covers the value all the way to
`create_model_package_group(ModelPackageGroupDescription=...)` and would also
catch a later rename of the hook parameter. The hook's own tests only ever pass
the default empty description, so a non-empty group description had no coverage
at any level before this.
Keeping the keyword form but restoring the wrong value fails that test alone
and leaves the other 9 in the file passing, so the test is tied to the defect
rather than to the keyword change. The existing `test_execute` assertion moved
to the keyword form the operator now uses.
`providers/amazon/tests/unit/amazon/aws/hooks/test_sagemaker.py` and
`providers/amazon/tests/unit/amazon/aws/operators/test_sagemaker_model.py` pass
together at 62 tests. `ruff format` and `ruff check` are clean on both changed
files.
No changelog entry by default: provider release managers regenerate the
changelog from `git log`.
No existing issue; `package_group_desc` returns no hits across open or
closed PRs and issues, so opening the PR directly.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes - Claude Code (Opus 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
