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

fix: Remove / when repoSubPath is not defined #887

Closed
wants to merge 1 commit into from

Conversation

afranzi
Copy link

@afranzi afranzi commented Sep 18, 2024

What issues does your PR fix?

  • This PR aims to fix the issue with Airflow 2.10.x where the scheduler compares the dag filelocation with the dag_folders path. The main issue is that the dags_folder setting ends up being /opt/airflow/dags/repo/ when the repoSubPath is empty, when the correct one should be /opt/airflow/dags/repo without the final slash. Otherwise the DAGs will disappear from the UI 🫠 .

The airflow code messing up with us is this one.

What does your PR do?

This PR makes the captures when the repoSubPath is not define in the values since the dags are already present in the root git folder

Checklist

For all Pull Requests

For releasing ONLY

@afranzi
Copy link
Author

afranzi commented Sep 18, 2024

Hello @thesuperzapper :)
Once you have time could you take a look into this PR? Right now is blocking us to update to the latest airflow version 🫠

@afranzi
Copy link
Author

afranzi commented Sep 20, 2024

On another note, this issue is also linked to this Airflow PR, since both needs to be merged to work properly in airflow 2.10.

@thesuperzapper
Copy link
Member

@afranzi in my testing, this does not fix the issue dags disappearing when using git-sync in Airflow 2.10.1.

It seems that Airflow 2.10.2 has reverted the problematic change. See the discussion in apache/airflow#42111, and the revert in apache/airflow#42220, so I suggest using 2.10.2 if you already upgraded.

Given this, I will close this issue and put a warning not to use 2.10.1 with the chart on the version matrix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants