Before deploying (both image-based an dags-based), check for recursive loops

[tatiana]

✍️ Is your feature request related to a problem? Please describe.

Airflow is very strict with symbolic links in the DAGs folder that lead to recursive loops: https://github.com/apache/airflow/blob/965d752443c6b389a40a40f1fb651be3517e5800/airflow/utils/file.py#L243

However, let's say someone accidentally deploys a "bad" folder (e.g. by using astro deploy --dags). To have Airflow raise a RuntimeError and stop parsing all DAGs, which may be critical to a business, could be considered an overreaction. To raise this exception can have a very high cost for business and is very disruptive to Airflow users, as recently experienced by at least one Astronomer customer.

This is an example of how I created a recursive loop and was able to reproduce the problem using Airflow standalone:

ln -s `pwd`/my-airflow-project/ `pwd`/my-airflow-project/dags/bla

We can adapt this to reproduce the problem with Astro CLI.

This issue was observed, for instance, in the ZenDesk #57833 issue.

I proposed this Airflow behavior is improved: https://github.com/apache/airflow/issues/40980, but it needs to be clarified if/when this would happen.

🧩 Describe the solution you'd like

Before building images or deploying the dags folder, Astro CLI could check for recursive loops in the DAGs folder and error.

🤔 Describe alternatives you've considered

Change the Airflow behaviour

Is your feature request specific to a particular Astronomer Platform?

Both

• [x] Astro
• [x] Software
• [ ] None/Unknown

[tatiana]

I learned the following from @sunkickr :

He asked if astro dev parse didn't catch the problem.

The command astro dev parse runs automatically with astro deploy unless it is skipped with 'astro deploy -f`. Some customers skip it because it doesn't work with all DAGs, but there is now a file where you can provide a list of DAGs to skip.

I (@tatiana ) believe several users use -f to avoid committing the files to Git before deploying.

It seems an option for the Astro CLI to capture this error would be to use astro deploy --parse -f - this will run the parse test and deploy uncommitted files.

I believe these are all valid suggestions that the customer could try. I'll try to get someone from the team to validate this.

[uranusjr]

avoid committing the files to Git before deploying

Should we have a way to accomodate this workflow without them needing to pass -f? Using the flag just for this sounds very wrong to me.

This is regardless of how we handle recursive loop detection.