Skip to content

AIP 84: Migrate GET ASSET EVENTS legacy API to fast API - #43881

Merged
pierrejeambrun merged 44 commits into
apache:mainfrom
astronomer:AIP84-get-asset-events
Nov 14, 2024
Merged

AIP 84: Migrate GET ASSET EVENTS legacy API to fast API #43881
pierrejeambrun merged 44 commits into
apache:mainfrom
astronomer:AIP84-get-asset-events

Conversation

@vatsrahul1001

@vatsrahul1001vatsrahul1001 commented Nov 11, 2024

Copy link
Copy Markdown
Contributor

Migrating the connexion API for GET ASSETS EVENTS to fastAPI.

Depends on #43783
related: #43845

Swagger Specs

imageimage

Response JSON

{
"asset_events": [
{
"id": 1,
"asset_id": 1,
"asset_uri": "s3://bucket/key/1",
"extra": {
"foo": "bar"
},
"source_task_id": "source_task_id",
"source_dag_id": "source_dag_id",
"source_run_id": "source_run_id_1",
"source_map_index": -1,
"created_dagruns": [
{
"run_id": "source_run_id_1",
"dag_id": "source_dag_id",
"logical_date": "2020-06-11T18:00:00Z",
"start_date": "2020-06-11T18:00:00Z",
"end_date": "2020-06-11T18:00:00Z",
"state": "success",
"data_interval_start": "2020-06-11T18:00:00Z",
"data_interval_end": "2020-06-11T18:00:00Z"
}
],
"timestamp": "2020-06-11T18:00:00Z"
},
{
"id": 2,
"asset_id": 2,
"uri": "s3://bucket/key/2",
"extra": {
"foo": "bar"
},
"source_task_id": "source_task_id",
"source_dag_id": "source_dag_id",
"source_run_id": "source_run_id_2",
"source_map_index": -1,
"created_dagruns": [
{
"run_id": "source_run_id_2",
"dag_id": "source_dag_id",
"logical_date": "2020-06-11T18:00:00Z",
"start_date": "2020-06-11T18:00:00Z",
"end_date": "2020-06-11T18:00:00Z",
"state": "success",
"data_interval_start": "2020-06-11T18:00:00Z",
"data_interval_end": "2020-06-11T18:00:00Z"
}
],
"timestamp": "2020-06-11T18:00:00Z"
}
],
"total_entries": 2
}

^ 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 newsfragments.

@boring-cyborgboring-cyborgBot added area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. labels Nov 11, 2024
@vatsrahul1001vatsrahul1001 changed the title Aip84 get asset eventsAIP 84: Migrate GETASSET EVENTS legacy API to fast API Nov 11, 2024
@vatsrahul1001vatsrahul1001 changed the title AIP 84: Migrate GETASSET EVENTS legacy API to fast API AIP 84: Migrate GET ASSET EVENTS legacy API to fast API Nov 11, 2024
@vatsrahul1001
vatsrahul1001 marked this pull request as draft November 11, 2024 12:37
@vatsrahul1001vatsrahul1001 added the legacy api Whether legacy API changes should be allowed in PR label Nov 11, 2024
Comment threadairflow/api_fastapi/core_api/routes/public/assets.py
Comment threadtests/api_fastapi/core_api/routes/public/test_assets.py
Comment threadtests/api_fastapi/core_api/routes/public/test_assets.py Outdated
@amoghrajesh
amoghrajesh self-requested a review November 13, 2024 14:06

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

Comment threadtests/api_fastapi/core_api/routes/public/test_assets.py

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice. Just a few suggestions

Do you mind rebasing, base branch has been merged, this will make the review easier.

I'll do a final review then.

You just did during my inital review, thanks :) checking again.

Comment threadairflow/api_fastapi/core_api/routes/public/assets.py
Comment threadairflow/api_fastapi/core_api/routes/public/assets.py Outdated
Comment threadairflow/api_fastapi/core_api/datamodels/assets.py Outdated
Comment threadairflow/api_fastapi/core_api/datamodels/assets.py Outdated

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A few more comments, overall looking good.

Comment threadairflow/api_fastapi/core_api/datamodels/assets.py Outdated
Comment threadtests/api_fastapi/core_api/routes/public/test_assets.py Outdated
Comment threadtests/api_fastapi/core_api/routes/public/test_assets.py Outdated
Comment threadtests/api_fastapi/core_api/routes/public/test_assets.py Outdated

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great, thanks!

One last small modification needed following #43932 and we can merge

Comment threadairflow/api_fastapi/core_api/routes/public/assets.py Outdated
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:APIAirflow's REST/HTTP APIarea:UIRelated to UI/UX. For Frontend Developers.legacy apiWhether legacy API changes should be allowed in PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@vatsrahul1001@pierrejeambrun@amoghrajesh