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

Do not mount DAG folder to scheduler in "standalone dag processor" mode in Chart #26474

Closed
2 tasks done
potiuk opened this issue Sep 19, 2022 · 4 comments
Closed
2 tasks done
Assignees
Labels
area:helm-chart Airflow Helm Chart kind:feature Feature Requests

Comments

@potiuk
Copy link
Member

potiuk commented Sep 19, 2022

Description

I believe with "standalone dag processor" completed in 2.4, Scheduler should not need a DAG folder to be mounted?

Should we disable the mount it in the helm chart in "standalone dag processor" mode similarly as we do for webserver in all cases?

Use case/motivation

Additional layer of security of the Scheduler - which would not have to have access to DAG code .

Related issues

Dicussed in #26008 (comment)

Are you willing to submit a PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@potiuk potiuk added kind:feature Feature Requests area:helm-chart Airflow Helm Chart labels Sep 19, 2022
@potiuk potiuk added this to the Airflow Helm Chart 1.7.0 milestone Sep 19, 2022
@potiuk
Copy link
Member Author

potiuk commented Sep 19, 2022

cc: @jedcunningham @ashb @mhenc @ephraimbuddy @uranusjr -> do you think of any reason why Scheduler in standalone dag processor mode would need DAG folder/code? I can't think of any, but maybe I forgot about somethig so I raised the issue (and happy to implement it before we release the new chart) - but I wanted to get your feedback on it.

@potiuk potiuk self-assigned this Sep 19, 2022
@mhenc
Copy link
Collaborator

mhenc commented Sep 19, 2022

No, I hope not - that was the whole idea of DagProcessor separation - to make Scheduler "trusted component" that runs without any interaction with a user code.
If there is still any dependency required, then we should fix it.

@jedcunningham
Copy link
Member

This was already done in #23711, or am I overlooking something?

@potiuk
Copy link
Member Author

potiuk commented Sep 19, 2022

Nope. you did not. It's I who overlooked it. 🤦

@potiuk potiuk closed this as completed Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:helm-chart Airflow Helm Chart kind:feature Feature Requests
Projects
None yet
Development

No branches or pull requests

3 participants