Skip to content

AIP-84 Migrate get connections to FastAPI API #42571#42782

Merged
pierrejeambrun merged 8 commits intoapache:mainfrom
bugraoz93:feat/42591/get-connections-to-fastapi
Oct 15, 2024
Merged

AIP-84 Migrate get connections to FastAPI API #42571#42782
pierrejeambrun merged 8 commits intoapache:mainfrom
bugraoz93:feat/42591/get-connections-to-fastapi

Conversation

@bugraoz93
Copy link
Contributor

closes: #42591


^ 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 Oct 6, 2024
@bugraoz93
Copy link
Contributor Author

Could someone please include legacy api label, I have got into the special checks merged from here #42758 :)

@gopidesupavan gopidesupavan added the legacy api Whether legacy API changes should be allowed in PR label Oct 6, 2024
@gopidesupavan
Copy link
Member

Could someone please include legacy api label, I have got into the special checks merged from here #42758 :)

done..

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.

Yes we need the model for the default fallback Model to look for columns.

Few minor suggestions.

@bugraoz93 bugraoz93 requested review from ashb and potiuk as code owners October 12, 2024 13:07
@pierrejeambrun pierrejeambrun force-pushed the feat/42591/get-connections-to-fastapi branch from 90b4f4c to b1a0c9f Compare October 15, 2024 14:55
@pierrejeambrun
Copy link
Member

I just rebased the branch and resolved merge conflicts. I think we are good to go.

@pierrejeambrun
Copy link
Member

Merging because we need that for other PRs. (dynamic order_by).

We can adjust with following PR if needed.

@pierrejeambrun pierrejeambrun merged commit 1ca6208 into apache:main Oct 15, 2024
@bugraoz93
Copy link
Contributor Author

Merging because we need that for other PRs. (dynamic order_by).

We can adjust with following PR if needed.

Make sense! I don't want to block anyone in the development flow. At least, you have already driven it and finalised the work so that it didn't block anyone for a long time.
For sure, I don't have any comments on the SortParam part. It looks great. I think one of the methods (get_primary_key_of_given_model_string) of the SortParam hasn't been used anywhere since the bespoke part was removed. I will remove the unused method in my next MR. I have started working on the next one PATCH a Connection already.

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]>
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
)

* 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

area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. legacy api Whether legacy API changes should be allowed in PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AIP-84 Migrate the public endpoint Get Connections to FastAPI

4 participants