Skip to content

fix: task-sdk AssetEventOperations.get to use alias_name when specified #52303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jun 27, 2025
Merged

fix: task-sdk AssetEventOperations.get to use alias_name when specified #52303

merged 2 commits into from
Jun 27, 2025

Conversation

jharriman
Copy link
Contributor

@jharriman jharriman commented Jun 26, 2025 •
edited
Loading

While using

@task
def my_task(inlet_events):
    asset_events = inlet_events[my_asset_alias]

I noticed that asset_events was empty, despite the asset event existing in the db. When inspecting AssetEventOperations.get I noticed that name was being passed as the alias name instead of alias_name (despite the elif block checking alias_name).

The checking the execution_api, it does appear to expect a name query parameter, but that parameter should align with the alias_name:

@router.get("/by-asset-alias")
def get_asset_event_by_asset_alias(
    name: Annotated[str, Query(description="The name of the Asset Alias")],
    session: SessionDep,
) -> AssetEventsResponse:
    return _get_asset_events_through_sql_clauses(
        join_clause=AssetEvent.source_aliases,
        where_clause=(AssetAliasModel.name == name),
        session=session,
    )

Added a test to assert the passed name matches the alias_name.

cc @Lee-W re: original PR here #45960

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg boring-cyborg
Copy link

boring-cyborg bot commented Jun 26, 2025

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 (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits 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: [email protected]
    Slack: https://s.apache.org/airflow-slack

@jharriman jharriman changed the title fix: AssetEventOperations.get to use alias_name when specified fix: task-sdk AssetEventOperations.get to use alias_name when specified Jun 26, 2025
@jharriman jharriman marked this pull request as ready for review June 26, 2025 19:11
@jharriman
Copy link
Contributor Author

@ashb @kaxil @amoghrajesh Long time user, first time contributor :) Hopefully this is a quick fix for an API parameter typo. Let me know how I can help get this merged. Thanks!

@kaxil kaxil added this to the Airflow 3.0.3 milestone Jun 26, 2025
@jharriman
Copy link
Contributor Author

Thanks for the ✅ @kaxil. Looks like the fix for 3.9 worked and the checks are all green.

Lee-W
Lee-W approved these changes Jun 27, 2025
View reviewed changes
Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

amoghrajesh
amoghrajesh approved these changes Jun 27, 2025
View reviewed changes
Copy link
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find @jharriman!

@amoghrajesh amoghrajesh added the backport-to-v3-0-test Mark PR with this label to backport to v3-0-test branch label Jun 27, 2025
@amoghrajesh amoghrajesh merged commit ac9e968 into apache:main Jun 27, 2025
73 checks passed
@boring-cyborg boring-cyborg
Copy link

boring-cyborg bot commented Jun 27, 2025

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

github-actions bot pushed a commit that referenced this pull request Jun 27, 2025
@jharriman
[v3-0-test] fix: task-sdk AssetEventOperations.get to use alias_name …
b8fe448 
…when specified (#52303)

* Fix AssetEventOperations.get to use alias_name when specified

* Use syntax compatible with python 3.9
(cherry picked from commit ac9e968)

Co-authored-by: Justyn Harriman <[email protected]>
@github-actions GitHub Actions
Copy link

Backport successfully created: v3-0-test

Status Branch Result
✅ v3-0-test PR Link

github-actions bot pushed a commit to astronomer/airflow that referenced this pull request Jun 27, 2025
@jharriman
[v3-0-test] fix: task-sdk AssetEventOperations.get to use alias_name …
cec4ad7 
…when specified (apache#52303)

* Fix AssetEventOperations.get to use alias_name when specified

* Use syntax compatible with python 3.9
(cherry picked from commit ac9e968)

Co-authored-by: Justyn Harriman <[email protected]>
Lee-W pushed a commit that referenced this pull request Jun 27, 2025
@github-actions @jharriman
[v3-0-test] fix: task-sdk AssetEventOperations.get to use alias_name …
644aa7c 
…when specified (#52303) (#52324)

* Fix AssetEventOperations.get to use alias_name when specified

* Use syntax compatible with python 3.9
(cherry picked from commit ac9e968)

Co-authored-by: Justyn Harriman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lee-W Lee-W Lee-W approved these changes

@kaxil kaxil kaxil approved these changes

@amoghrajesh amoghrajesh amoghrajesh approved these changes

@ashb ashb Awaiting requested review from ashb ashb is a code owner

Assignees
No one assigned
Labels
area:task-sdk backport-to-v3-0-test Mark PR with this label to backport to v3-0-test branch
Projects
None yet
Milestone
Airflow 3.0.3
Development

Successfully merging this pull request may close these issues.
4 participants
@jharriman @Lee-W @kaxil @amoghrajesh