Skip to content

Conversation

@jason810496
Copy link
Member

related: #42360


^ 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 Feb 28, 2025
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, one nit that should be good to figure out. Basically the permission decorator should not change the openapi spec. (beside the auth scheme)

@jason810496 jason810496 force-pushed the feature/AIP-84/auth/config branch from c9bbbef to 87f0452 Compare March 3, 2025 13:01
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, need rebase.

@jason810496 jason810496 force-pushed the feature/AIP-84/auth/config branch from 87f0452 to b18f228 Compare March 3, 2025 14:28
@jedcunningham jedcunningham added the AIP-84 Modern Rest API label Mar 4, 2025
@jason810496 jason810496 force-pushed the feature/AIP-84/auth/config branch from b18f228 to 77e94c0 Compare March 4, 2025 11:58
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.

Need rebase to solve conflicts.

@jason810496 jason810496 force-pushed the feature/AIP-84/auth/config branch from 77e94c0 to 94e326c Compare March 4, 2025 13:02
@pierrejeambrun
Copy link
Member

The CI error persists. That's weird

@potiuk
Copy link
Member

potiuk commented Mar 4, 2025

it's not the config that is problematic now:

Triggering a DAG run
Exception when calling DAGRunAPI->post_dag_run: (422)
Reason: Unprocessable Entity
HTTP response headers: HTTPHeaderDict({'date': 'Tue, 04 Mar 2025 14:38:27 GMT', 
'server': 'uvicorn', 'content-length': '156', 'content-type': 
'application/json', 'vary': 'Accept-Encoding'})
HTTP response body: 
{"detail":[{"type":"missing","loc":["body","logical_date"],"msg":"Field 
required","input":{"dag_run_id":"some_test_run_8a505b32352c44499cf6560a1d1a9665"
}}]}


Exception when calling DAGRunAPI->post_dag_run: (401)
Reason: Unauthorized
HTTP response headers: HTTPHeaderDict({'date': 'Tue, 04 Mar 2025 14:38:27 GMT', 
'server': 'uvicorn', 'www-authenticate': 'Bearer', 'content-length': '30', 
'content-type': 'application/json', 'vary': 'Accept-Encoding'})
HTTP response body: {"detail":"Not authenticated"}

@jason810496
Copy link
Member Author

jason810496 commented Mar 5, 2025

it's not the config that is problematic now:

Thanks to point it out, let me take a look at python client test and fix it.

@jason810496 jason810496 force-pushed the feature/AIP-84/auth/config branch from 94e326c to 87736b5 Compare March 5, 2025 14:55
@pierrejeambrun pierrejeambrun added the full tests needed We need to run full set of tests for this PR to merge label Mar 5, 2025
@pierrejeambrun pierrejeambrun reopened this Mar 6, 2025
@jason810496 jason810496 force-pushed the feature/AIP-84/auth/config branch from 87736b5 to df667fb Compare March 6, 2025 13:32
@pierrejeambrun
Copy link
Member

I have a fix for Tests / CI image checks / Test Python API client (pull_request) basically that is not specifically related to this PR. The test is just broken at the moment. (passing on main but not testing anything)

@pierrejeambrun
Copy link
Member

#47460

@Lee-W Lee-W force-pushed the feature/AIP-84/auth/config branch from df667fb to 12bdc65 Compare March 7, 2025 07:56
@Lee-W
Copy link
Member

Lee-W commented Mar 7, 2025

As this has been approved and full test passed, I'm merging this one

@Lee-W Lee-W merged commit ab3a186 into apache:main Mar 7, 2025
89 checks passed
nailo2c pushed a commit to nailo2c/airflow that referenced this pull request Apr 4, 2025
* AIP-84 | Add Auth for Config

* Fix requires_access_configuration schema

* Refactor requires_access_configuration with lambda
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. full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants