You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "weiyu1029 (via GitHub)" <gi...@apache.org> on 2026/09/02 18:22:01 UTC
[PR] Fix SageMaker Unified Studio trigger hook config [airflow]
weiyu1029 opened a new pull request, #72453:
URL: https://github.com/apache/airflow/pull/72453
<!-- SPDX-License-Identifier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->
### Description
Forward `region_name`, `verify`, and `botocore_config` from
`SageMakerUnifiedStudioNotebookOperator` to
`SageMakerUnifiedStudioNotebookTrigger` when execution is deferred.
Previously, these AWS hook configuration values were not forwarded to the
trigger. As a result, the triggerer could construct its hook using different
AWS configuration from the operator that originally started the notebook run,
including potentially polling a different AWS region.
This change ensures that the deferred trigger receives and serializes the same
AWS hook configuration used by the operator.
### Tests
Extended the deferrable operator unit test to verify that:
- `region_name` survives trigger serialization.
- `verify` survives trigger serialization.
- `botocore_config` survives trigger serialization.
- The derived `waiter_max_attempts` behavior remains unchanged.
Tested locally with:
```bash
breeze testing providers-tests providers/amazon/tests/unit/amazon/aws/operators/test_sagemaker_unified_studio_notebook.py
breeze testing providers-tests providers/amazon/tests/unit/amazon/aws/triggers/test_sagemaker_unified_studio_notebook.py
---
##### Was generative AI tooling used to co-author this PR?
<!--
If generative AI tooling has been used in the process of authoring this PR, please
change below checkbox to `[X]` followed by the name of the tool, uncomment the "Generated-by".
-->
- [X] Yes (please specify the tool below)
<!-- Generated-by: ChatGPT following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
-->
---
* Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment: `{pr_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments). You can add this file in a follow-up commit after the PR is created so you know the PR number.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix SageMaker Unified Studio trigger hook config [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck merged PR #72453:
URL: https://github.com/apache/airflow/pull/72453
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix SageMaker Unified Studio trigger hook config [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #72453:
URL: https://github.com/apache/airflow/pull/72453#issuecomment-5540448670
Awesome work, congrats on your first merged pull request! You are invited to check our [Issue Tracker](https://github.com/apache/airflow/issues) for additional contributions.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix SageMaker Unified Studio trigger hook config [airflow]
Posted by "SameerMesiah97 (via GitHub)" <gi...@apache.org>.
SameerMesiah97 commented on code in PR #72453:
URL: https://github.com/apache/airflow/pull/72453#discussion_r3917907037
##########
providers/amazon/tests/unit/amazon/aws/operators/test_sagemaker_unified_studio_notebook.py:
##########
@@ -272,18 +279,30 @@ def test_execute_deferrable(self, mock_hook_prop):
deferrable=True,
waiter_delay=20,
timeout_configuration={"runTimeoutInMinutes": 120},
+ region_name="us-west-2",
+ verify="/tmp/custom-ca.pem",
+ botocore_config=botocore_config,
)
with pytest.raises(TaskDeferred) as exc_info:
op.execute(_make_context())
trigger = exc_info.value.trigger
+
assert isinstance(trigger, SageMakerUnifiedStudioNotebookTrigger)
assert trigger.notebook_run_id == NOTEBOOK_RUN_ID
assert trigger.domain_identifier == DOMAIN_ID
assert trigger.owning_project_identifier == PROJECT_ID
assert trigger.waiter_delay == 20
assert trigger.timeout_configuration == {"runTimeoutInMinutes": 120}
+
+ serialized_payload = trigger.serialize()[1]
Review Comment:
nit: _, kwargs = trigger.serialize() would be cleaner but it is fine as is.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix SageMaker Unified Studio trigger hook config [airflow]
Posted by "SameerMesiah97 (via GitHub)" <gi...@apache.org>.
SameerMesiah97 commented on PR #72453:
URL: https://github.com/apache/airflow/pull/72453#issuecomment-5515966712
> The review feedback has been addressed in [efb4e00](https://github.com/apache/airflow/commit/efb4e00628679f4d15ea565dc34da42c12e22326), and both targeted test suites pass locally. Could a maintainer please approve the three pending CI workflows? Thank you!
@jroachgolf84
could you tag the maintainer or maintainers sponsoring the contributor workshop?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix SageMaker Unified Studio trigger hook config [airflow]
Posted by "weiyu1029 (via GitHub)" <gi...@apache.org>.
weiyu1029 commented on PR #72453:
URL: https://github.com/apache/airflow/pull/72453#issuecomment-5515839778
The review feedback has been addressed in efb4e00, and both targeted test suites pass locally. Could a maintainer please approve the three pending CI workflows? Thank you!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix SageMaker Unified Studio trigger hook config [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on code in PR #72453:
URL: https://github.com/apache/airflow/pull/72453#discussion_r3917989131
##########
providers/amazon/tests/unit/amazon/aws/operators/test_sagemaker_unified_studio_notebook.py:
##########
@@ -272,18 +279,30 @@ def test_execute_deferrable(self, mock_hook_prop):
deferrable=True,
waiter_delay=20,
timeout_configuration={"runTimeoutInMinutes": 120},
+ region_name="us-west-2",
+ verify="/tmp/custom-ca.pem",
+ botocore_config=botocore_config,
)
with pytest.raises(TaskDeferred) as exc_info:
op.execute(_make_context())
trigger = exc_info.value.trigger
+
assert isinstance(trigger, SageMakerUnifiedStudioNotebookTrigger)
assert trigger.notebook_run_id == NOTEBOOK_RUN_ID
assert trigger.domain_identifier == DOMAIN_ID
assert trigger.owning_project_identifier == PROJECT_ID
assert trigger.waiter_delay == 20
assert trigger.timeout_configuration == {"runTimeoutInMinutes": 120}
+
+ serialized_payload = trigger.serialize()[1]
Review Comment:
+1 here
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix SageMaker Unified Studio trigger hook config [airflow]
Posted by "weiyu1029 (via GitHub)" <gi...@apache.org>.
weiyu1029 commented on code in PR #72453:
URL: https://github.com/apache/airflow/pull/72453#discussion_r3918232485
##########
providers/amazon/tests/unit/amazon/aws/operators/test_sagemaker_unified_studio_notebook.py:
##########
@@ -272,18 +279,30 @@ def test_execute_deferrable(self, mock_hook_prop):
deferrable=True,
waiter_delay=20,
timeout_configuration={"runTimeoutInMinutes": 120},
+ region_name="us-west-2",
+ verify="/tmp/custom-ca.pem",
+ botocore_config=botocore_config,
)
with pytest.raises(TaskDeferred) as exc_info:
op.execute(_make_context())
trigger = exc_info.value.trigger
+
assert isinstance(trigger, SageMakerUnifiedStudioNotebookTrigger)
assert trigger.notebook_run_id == NOTEBOOK_RUN_ID
assert trigger.domain_identifier == DOMAIN_ID
assert trigger.owning_project_identifier == PROJECT_ID
assert trigger.waiter_delay == 20
assert trigger.timeout_configuration == {"runTimeoutInMinutes": 120}
+
+ serialized_payload = trigger.serialize()[1]
Review Comment:
Addressed in efb4e00628 — switched to `_, kwargs = trigger.serialize()` and updated the assertions. Both related test files pass locally.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org