Connections API is able to patch all fields correctly #48109
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jscheffl
requested review from
ephraimbuddy,
pierrejeambrun and
rawwar
as code owners
ashb
reviewed
Mar 23, 2025
airflow-core/src/airflow/api_fastapi/core_api/routes/public/connections.py
Outdated
Show resolved
Hide resolved
Member
pierrejeambrun
left a comment
pierrejeambrun
left a comment
There was a problem hiding this comment.
Nice, thanks for the catch.
airflow-core/src/airflow/api_fastapi/core_api/services/public/connections.py
Show resolved
Hide resolved
jscheffl
force-pushed
the
bugfix/connections-api-patch-all-fields
branch
from
0ec69f1 to
1441339
Compare
Member
pierrejeambrun
left a comment
pierrejeambrun
left a comment
There was a problem hiding this comment.
Yep, sounds good to me.
Just a few nits and one test need fixing
airflow-core/src/airflow/api_fastapi/core_api/routes/public/connections.py
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/services/public/connections.py
Outdated
Show resolved
Hide resolved
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
Show resolved
Hide resolved
jscheffl
force-pushed
the
bugfix/connections-api-patch-all-fields
branch
from
1441339 to
7f187e5
Compare
Member
There was a problem hiding this comment.
I think we removed the model_fields_set and this is the source of confusion.
When no 'mask' is passed to the function, only fields that are explicitely specified in the payload (even if passed to 'None') should be set. Other missing fields from the partial body should not be updated. I tried locally and we observe the opposite in this current state.
airflow-core/src/airflow/api_fastapi/core_api/services/public/connections.py
Outdated
Show resolved
Hide resolved
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
Show resolved
Hide resolved
jscheffl
force-pushed
the
bugfix/connections-api-patch-all-fields
branch
from
7f187e5 to
a7ff401
Compare
jscheffl
force-pushed
the
bugfix/connections-api-patch-all-fields
branch
from
a7ff401 to
dca8763
Compare
pierrejeambrun
approved these changes
Mar 31, 2025
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
Show resolved
Hide resolved
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
Show resolved
Hide resolved
jscheffl
force-pushed
the
bugfix/connections-api-patch-all-fields
branch
from
dca8763 to
d9f160d
Compare
jscheffl
force-pushed
the
bugfix/connections-api-patch-all-fields
branch
from
d9f160d to
587852e
Compare
kaxil
added a commit
to astronomer/airflow
that referenced
this pull request
Mar 31, 2025
I am sure this wasn't intended to be added in apache#48109
pierrejeambrun
pushed a commit
that referenced
this pull request
Mar 31, 2025
shubham-pyc
pushed a commit
to shubham-pyc/airflow
that referenced
this pull request
Apr 2, 2025
* Connections API is able to patch all fields correctly * Fix implementation and pytest, apply to bulk service as well * Consolidate Pydantic to ORM update in connections API * Review feedback - genralize via model_dump where possible * Fix pytest * Review feedback * Review feedback, extended pytests * Add a explicit test if None is passed in body
shubham-pyc
pushed a commit
to shubham-pyc/airflow
that referenced
this pull request
Apr 2, 2025
I am sure this wasn't intended to be added in apache#48109
nailo2c
pushed a commit
to nailo2c/airflow
that referenced
this pull request
Apr 4, 2025
* Connections API is able to patch all fields correctly * Fix implementation and pytest, apply to bulk service as well * Consolidate Pydantic to ORM update in connections API * Review feedback - genralize via model_dump where possible * Fix pytest * Review feedback * Review feedback, extended pytests * Add a explicit test if None is passed in body
nailo2c
pushed a commit
to nailo2c/airflow
that referenced
this pull request
Apr 4, 2025
I am sure this wasn't intended to be added in apache#48109
diogotrodrigues
pushed a commit
to diogotrodrigues/airflow
that referenced
this pull request
Apr 6, 2025
* Connections API is able to patch all fields correctly * Fix implementation and pytest, apply to bulk service as well * Consolidate Pydantic to ORM update in connections API * Review feedback - genralize via model_dump where possible * Fix pytest * Review feedback * Review feedback, extended pytests * Add a explicit test if None is passed in body
diogotrodrigues
pushed a commit
to diogotrodrigues/airflow
that referenced
this pull request
Apr 6, 2025
I am sure this wasn't intended to be added in apache#48109
simonprydden
pushed a commit
to simonprydden/airflow
that referenced
this pull request
Apr 8, 2025
* Connections API is able to patch all fields correctly * Fix implementation and pytest, apply to bulk service as well * Consolidate Pydantic to ORM update in connections API * Review feedback - genralize via model_dump where possible * Fix pytest * Review feedback * Review feedback, extended pytests * Add a explicit test if None is passed in body
simonprydden
pushed a commit
to simonprydden/airflow
that referenced
this pull request
Apr 8, 2025
I am sure this wasn't intended to be added in apache#48109
kosteev
pushed a commit
to GoogleCloudPlatform/composer-airflow
that referenced
this pull request
May 29, 2025
I am sure this wasn't intended to be added in apache/airflow#48109 GitOrigin-RevId: 71bbaebfea7d1a582c3ef03b07d8cc39a88928ef
kosteev
pushed a commit
to GoogleCloudPlatform/composer-airflow
that referenced
this pull request
Sep 24, 2025
I am sure this wasn't intended to be added in apache/airflow#48109 GitOrigin-RevId: 71bbaebfea7d1a582c3ef03b07d8cc39a88928ef
kosteev
pushed a commit
to GoogleCloudPlatform/composer-airflow
that referenced
this pull request
Oct 22, 2025
I am sure this wasn't intended to be added in apache/airflow#48109 GitOrigin-RevId: 71bbaebfea7d1a582c3ef03b07d8cc39a88928ef
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
During implementation of #48102 we saw that the patch API for connections is not working correctly.
This PR fixes the problem - reason is that pydantic fields to not map 1:1 to API fields - as well as extends pytests to cover the difference