Skip to content

Add failure-path and serialization tests for StepFunctionsExecutionCompleteTrigger - #72570

Merged
vincbeck merged 1 commit into
apache:mainfrom
bingqin2:add-step-function-trigger-tests
Sep 9, 2026
Merged

vincbeck merged 1 commit into
apache:mainfrom
bingqin2:add-step-function-trigger-tests

Conversation

@bingqin2

@bingqin2 bingqin2 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

The region_name fix originally proposed here was merged in the meantime via #72625, so this PR is re-scoped to the parts that PR did not cover:

  • test_run_failure: the trigger's error path, i.e. async_wait raising AirflowException yields a TriggerEvent with status="error", the message, and the execution ARN
  • test_serialization_with_verify_and_botocore_config: verify and botocore_config round-trip through serialize()
  • test_serialization_omits_unset_hook_params: region_name, verify, and botocore_config are pruned from the serialized kwargs when not set; before Fix region_name being ignored by the Step Functions execution trigger #72625 the trigger always serialized region_name, even as None
  • documents the region_name parameter in the trigger docstring

The tests merged in #72625 are left unchanged; these are additive.

related: #72625


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: Claude Code (Claude Fable 5.1) following the guidelines. I reviewed and understand all changes; the tests were run locally.


🤖 Generated with Claude Code

@boring-cyborg boring-cyborg Bot added area:providers provider:amazon AWS/Amazon - related issues labels Sep 5, 2026
@boring-cyborg

boring-cyborg Bot commented Sep 5, 2026

Copy link
Copy Markdown

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example Dag that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@bingqin2
bingqin2 marked this pull request as ready for review September 6, 2026 03:11
@bingqin2
bingqin2 requested a review from o-nikolas as a code owner September 6, 2026 03:11
…mpleteTrigger

The region_name fix originally proposed in this PR landed via apache#72625. This adds the remaining coverage: the error TriggerEvent path, verify/botocore_config serialization, pruning of unset hook parameters, and the region_name docstring.
@bingqin2
bingqin2 force-pushed the add-step-function-trigger-tests branch from a8360a7 to 9b2bb19 Compare September 9, 2026 16:18
@bingqin2 bingqin2 changed the title Fix region_name being dropped by StepFunctionsExecutionCompleteTrigger and add its tests Add failure-path and serialization tests for StepFunctionsExecutionCompleteTrigger Sep 9, 2026
@bingqin2

bingqin2 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the approval @vincbeck. Since the same fix landed via #72625 while this one was waiting, I have rebased and re-scoped this PR to what is still missing after that merge: the trigger's failure-path test, serialization tests for verify/botocore_config and for the pruning of unset hook parameters, and the region_name docstring entry. The tests from #72625 are untouched. CI is re-running now.

@vincbeck
vincbeck merged commit 1b19d73 into apache:main Sep 9, 2026
82 checks passed
@boring-cyborg

boring-cyborg Bot commented Sep 9, 2026

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@bingqin2
bingqin2 deleted the add-step-function-trigger-tests branch September 10, 2026 03:51
imrichardwu pushed a commit to imrichardwu/airflow that referenced this pull request Sep 11, 2026
…mpleteTrigger (apache#72570)

The region_name fix originally proposed in this PR landed via apache#72625. This adds the remaining coverage: the error TriggerEvent path, verify/botocore_config serialization, pruning of unset hook parameters, and the region_name docstring.
xvega pushed a commit to xvega/airflow that referenced this pull request Sep 13, 2026
…mpleteTrigger (apache#72570)

The region_name fix originally proposed in this PR landed via apache#72625. This adds the remaining coverage: the error TriggerEvent path, verify/botocore_config serialization, pruning of unset hook parameters, and the region_name docstring.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:amazon AWS/Amazon - related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants