Skip to content

Conversation

@rawwar
Copy link
Contributor

@rawwar rawwar commented Mar 8, 2025

related to #42360

  • dag_run
  • dag_parsing
  • dag_source
  • dag_stats
  • dag_warning
  • ti
  • xcom

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. labels Mar 8, 2025
@rawwar rawwar marked this pull request as ready for review March 8, 2025 11:35
@Lee-W Lee-W mentioned this pull request Mar 10, 2025
47 tasks
@Lee-W Lee-W self-requested a review March 11, 2025 09:29
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.

Great! Looking good thanks.

Just a few suggestions, and need rebasing + CI fixing, should be good to go after that.

@pierrejeambrun
Copy link
Member

For CI authenticated request on the K8S tests, you can take a look at #47433 that encountered the same problem I believe.

@pierrejeambrun
Copy link
Member

Also you'll need to wait for #47433 because it introduces EditableDagsFilterDep and ReadableDagsFilterDep that you will need for multiple endpoints. For instance dag_warnings, task_instances etc....

Basically it's up to the view to do extra filtering on readable/writable dags for specific endpoints. (there is a handful of endpoints to update)

@Lee-W Lee-W force-pushed the kalyan/AIP-84/auth/multiple_dag_endpoints branch from 62c17c0 to a3eef56 Compare March 11, 2025 14:43
@Lee-W
Copy link
Member

Lee-W commented Mar 11, 2025

just resolve the conflict and rebase from the latest main. we should have the required methods now

@rawwar
Copy link
Contributor Author

rawwar commented Mar 11, 2025

just resolve the conflict and rebase from the latest main. we should have the required methods now

I'll update based on feedback in sometime

@Lee-W Lee-W force-pushed the kalyan/AIP-84/auth/multiple_dag_endpoints branch from 5eedfba to db8ac76 Compare March 12, 2025 09: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.

Looks good thanks

@pierrejeambrun
Copy link
Member

pierrejeambrun commented Mar 12, 2025

@jason810496 how can this be flaky ? I thought the session was authenticated
https://github.com/apache/airflow/actions/runs/13817174257/job/38655368903?pr=47532

Also forbidden means that the token is provided but not correct. (maybe the signature key is not the same at generation time and at verification time and need to be set in the config). => we do not explicitly set it in the config. Therefore we generate one and set it everytime which might end up causing different keys to be used...

@jason810496
Copy link
Member

@jason810496 how can this be flaky ? I thought the session was authenticated https://github.com/apache/airflow/actions/runs/13817174257/job/38655368903?pr=47532

maybe the signature key is not the same at generation time and at verification time and need to be set in the config

Good point, I will create another PR that set the following env in configMap explicitly

  • auth_jwt_secret
  • auth_jwt_expiration_time ( I think we don't need to set this, since the default value is 86400, the CI will not exceed 1 day )

@Lee-W
Copy link
Member

Lee-W commented Mar 13, 2025

all green and all comments resolved. merging

@Lee-W Lee-W merged commit df0b6c3 into apache:main Mar 13, 2025
89 checks passed
@pierrejeambrun
Copy link
Member

That would be great @jason810496 maybe that will help stabilizing, ty

@jason810496
Copy link
Member

That would be great @jason810496 maybe that will help stabilizing, ty

Yeah, I'm already working on it but not able to finish it today. I will raise the PR tomorrow.

@ashb
Copy link
Member

ashb commented Mar 13, 2025

@jason810496 how can this be flaky ? I thought the session was authenticated https://github.com/apache/airflow/actions/runs/13817174257/job/38655368903?pr=47532

maybe the signature key is not the same at generation time and at verification time and need to be set in the config

Good point, I will create another PR that set the following env in configMap explicitly

* `auth_jwt_secret`

* `auth_jwt_expiration_time` ( I think we don't need to set this, since the default value is `86400`, the CI will not exceed 1 day )

These will change names soon in #46981

@ashb
Copy link
Member

ashb commented Mar 13, 2025

@jason810496 how can this be flaky ? I thought the session was authenticated https://github.com/apache/airflow/actions/runs/13817174257/job/38655368903?pr=47532

Also forbidden means that the token is provided but not correct. (maybe the signature key is not the same at generation time and at verification time and need to be set in the config). => we do not explicitly set it in the config. Therefore we generate one and set it everytime which might end up causing different keys to be used...

#47739 most likely

nailo2c pushed a commit to nailo2c/airflow that referenced this pull request Apr 4, 2025
…g_run, dag_parsing (apache#47532)

* add dag_parsing

* add dag_run

* add dag_source

* add dag_stats

* add dag_warning

* add ti

* add xcom

* add dag_parsing

* add dag_run

* add dag_source

* add dag_stats

* add dag_warning

* add ti

* add xcom

* pr feedback

* use dag run filter

* add dag_parsing

* add dag_run

* add dag_source

* add dag_stats

* add dag_warning

* add ti

* add xcom

* pr feedback

* use dag run filter

* ci failures

* add dag_id filters

---------

Co-authored-by: Wei Lee <weilee.rx@gmail.com>
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants