Skip to content

Conversation

@abhishekbhakat
Copy link
Contributor

closes: #49607

Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Looks good to me ( maybe rebase later once the CI is fixed. )

Follow up:

I think we can add pre-commit for preventing duplicate operationId.
Maybe not a new one, adding validate_spec in generate-openapi-spec pre-commit will be enough I think.

@bbovenzi
Copy link
Contributor

Can you check that BlockingDeps.tsx in the UI still works?

It should render in the "Details" tab when you have a task instance selected.

Copy link
Contributor

@bugraoz93 bugraoz93 left a comment

Choose a reason for hiding this comment

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

Looks good! As mentioned, revase, fix CI, and manual tests would be awesome. Thanks for the pr!

@abhishekbhakat
Copy link
Contributor Author

Can you check that BlockingDeps.tsx in the UI still works?

It should render in the "Details" tab when you have a task instance selected.

Screenshot 2025-04-23 at 16 00 57

@bbovenzi this is what I got

@abhishekbhakat
Copy link
Contributor Author

abhishekbhakat commented Apr 23, 2025

@bugraoz93

  • The CI failing is not related to this PR. I'll have to just wait till they are fixed.
  • To add the tests, I might need to add the openapi-spec-validator in the deps. Let me know if that's ok I'll add it.

Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

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

Looks good. Tested locally and working.

I don't think we need unit test for that. I think what Bugra was referring to manual testing. (verifying in the UI that everything looks correct for marking a mapped task and normal task as success or failure, as well as updating the task notes).

@bugraoz93
Copy link
Contributor

bugraoz93 commented Apr 23, 2025

@bugraoz93

  • The CI failing is not related to this PR. I'll have to just wait till they are fixed.
  • To add the tests, I might need to add the openapi-spec-validator in the deps. Let me know if that's ok I'll add it.

Yeah, I wasn't on the computer, checked it now :) Yes, as Pierre mentioned, I meant the manual test

@abhishekbhakat
Copy link
Contributor Author

abhishekbhakat commented Apr 23, 2025

Added validation of openapi spec logic in the run_update_fastapi_api_spec.py so now it runs with pre-commit hook generate-openapi-spec

CC @jason810496

Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

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

Looks good thanks.

@pierrejeambrun pierrejeambrun merged commit 1cd8614 into apache:main Apr 24, 2025
96 checks passed
prabhusneha pushed a commit to astronomer/airflow that referenced this pull request Apr 25, 2025
apache#49608)

* Rename operation IDs for task instance endpoints to include map index for openapi spec validator

* refactor: Update TaskInstanceService methods and types to use map index for dependencies

* Add OpenAPI validation to FastAPI spec generation script

* refactor: streamline OpenAPI validation process by removing redundant exit checks
@kaxil kaxil modified the milestones: Airflow 3.0.1, Airflow 3.0.2 May 16, 2025
kaxil pushed a commit that referenced this pull request May 16, 2025
#49608)

* Rename operation IDs for task instance endpoints to include map index for openapi spec validator

* refactor: Update TaskInstanceService methods and types to use map index for dependencies

* Add OpenAPI validation to FastAPI spec generation script

* refactor: streamline OpenAPI validation process by removing redundant exit checks

(cherry picked from commit 1cd8614)
kaxil pushed a commit that referenced this pull request Jun 3, 2025
#49608)

* Rename operation IDs for task instance endpoints to include map index for openapi spec validator

* refactor: Update TaskInstanceService methods and types to use map index for dependencies

* Add OpenAPI validation to FastAPI spec generation script

* refactor: streamline OpenAPI validation process by removing redundant exit checks

(cherry picked from commit 1cd8614)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unique operationId in openapi

6 participants