Skip to content
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.

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

AIP-84 Migrate delete a connection to FastAPI API #42571

Conversation

bugraoz93
Copy link
Collaborator

closes: #42559


^ 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-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. labels Sep 29, 2024
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.

Nice 👍.

A couple of really minor nits / suggestions, but that is already in a mergeable state :)

# Conflicts:
#	airflow/ui/openapi-gen/queries/common.ts
#	airflow/ui/openapi-gen/queries/queries.ts
@bugraoz93
Copy link
Collaborator Author

Nice 👍.

A couple of really minor nits / suggestions, but that is already in a mergeable state :)

Thanks for all of your comments! :) All are valuable 🙏

@pierrejeambrun
Copy link
Member

pierrejeambrun commented Oct 1, 2024

Thanks. Can you rebase the PR, we just merged a new change that updates the operationId generated in the openapi spec, this way the front-end generated code has shorter, more meaningful names instead of the current really long ones that are generated.

Basically you need to use the AirflowRouter instead of the fastapi.APIRouter (this way the operationId is inferred based on the function name of the operation), then regenerate the openapi spec and the frontend code with the pre-commit hooks :)

I think it might also solves the conflicts.

# Conflicts:
#	airflow/api_fastapi/views/public/__init__.py
#	airflow/ui/openapi-gen/requests/services.gen.ts
@bugraoz93
Copy link
Collaborator Author

I fixed the conflict and generated files again. Great addition, thanks! Indeed, they were quite long :)

@pierrejeambrun
Copy link
Member

Thanks @bugraoz93

@pierrejeambrun pierrejeambrun merged commit 0c911a9 into apache:main Oct 2, 2024
52 checks passed
@pierrejeambrun pierrejeambrun added the AIP-84 Modern Rest API label Oct 2, 2024
pierrejeambrun added a commit that referenced this pull request Oct 15, 2024
* Make SortParam parent for Model Specific SortParams, Include get connections endpoint to fastapi

* Change depends() method regular method in SortParam due to parent class already have abstract

* Remove subclass, get default order_by from primary key, change alias strategy for backcompat

* pre-commit hooks

* Dynamic return value of SortParam generated within openapi specs and removed unnecessary attribute mapping keys

* Include connection_id to attr_mapping again

* Dynamic depends with correct documentation

* Add more tests

---------

Co-authored-by: pierrejeambrun <[email protected]>
R7L208 pushed a commit to R7L208/airflow that referenced this pull request Oct 17, 2024
)

* Make SortParam parent for Model Specific SortParams, Include get connections endpoint to fastapi

* Change depends() method regular method in SortParam due to parent class already have abstract

* Remove subclass, get default order_by from primary key, change alias strategy for backcompat

* pre-commit hooks

* Dynamic return value of SortParam generated within openapi specs and removed unnecessary attribute mapping keys

* Include connection_id to attr_mapping again

* Dynamic depends with correct documentation

* Add more tests

---------

Co-authored-by: pierrejeambrun <[email protected]>
joaopamaral pushed a commit to joaopamaral/airflow that referenced this pull request Oct 21, 2024
* Include connections router and migrate delete a connection endpoint to fastapi

* Mark tests as db_test

* Use only pyfixture session

* make method async

* setup method to setup_attrs

* Convert APIRouter tags, make setup method unified

* Use AirflowRouter over fastapi.APIRouter
harjeevanmaan pushed a commit to harjeevanmaan/airflow that referenced this pull request Oct 23, 2024
)

* Make SortParam parent for Model Specific SortParams, Include get connections endpoint to fastapi

* Change depends() method regular method in SortParam due to parent class already have abstract

* Remove subclass, get default order_by from primary key, change alias strategy for backcompat

* pre-commit hooks

* Dynamic return value of SortParam generated within openapi specs and removed unnecessary attribute mapping keys

* Include connection_id to attr_mapping again

* Dynamic depends with correct documentation

* Add more tests

---------

Co-authored-by: pierrejeambrun <[email protected]>
PaulKobow7536 pushed a commit to PaulKobow7536/airflow that referenced this pull request Oct 24, 2024
)

* Make SortParam parent for Model Specific SortParams, Include get connections endpoint to fastapi

* Change depends() method regular method in SortParam due to parent class already have abstract

* Remove subclass, get default order_by from primary key, change alias strategy for backcompat

* pre-commit hooks

* Dynamic return value of SortParam generated within openapi specs and removed unnecessary attribute mapping keys

* Include connection_id to attr_mapping again

* Dynamic depends with correct documentation

* Add more tests

---------

Co-authored-by: pierrejeambrun <[email protected]>
ellisms pushed a commit to ellisms/airflow that referenced this pull request Nov 13, 2024
* Include connections router and migrate delete a connection endpoint to fastapi

* Mark tests as db_test

* Use only pyfixture session

* make method async

* setup method to setup_attrs

* Convert APIRouter tags, make setup method unified

* Use AirflowRouter over fastapi.APIRouter
ellisms pushed a commit to ellisms/airflow that referenced this pull request Nov 13, 2024
)

* Make SortParam parent for Model Specific SortParams, Include get connections endpoint to fastapi

* Change depends() method regular method in SortParam due to parent class already have abstract

* Remove subclass, get default order_by from primary key, change alias strategy for backcompat

* pre-commit hooks

* Dynamic return value of SortParam generated within openapi specs and removed unnecessary attribute mapping keys

* Include connection_id to attr_mapping again

* Dynamic depends with correct documentation

* Add more tests

---------

Co-authored-by: pierrejeambrun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AIP-84 Modern Rest API area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AIP-84 Migrate the public endpoint Delete Connection to FastAPI
2 participants