You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "pierrejeambrun (via GitHub)" <gi...@apache.org> on 2023/08/12 21:23:29 UTC
[GitHub] [airflow] pierrejeambrun opened a new pull request, #33357: Fix/33310 dag file processor deleted dags with subdir
pierrejeambrun opened a new pull request, #33357:
URL: https://github.com/apache/airflow/pull/33357
closes: https://github.com/apache/airflow/issues/33310
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #33357: Fix DagFileProcessor interfering with dags outside its ``processor_subdir``
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #33357:
URL: https://github.com/apache/airflow/pull/33357#issuecomment-1676497984
Nice catch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on a diff in pull request #33357: Fix DagFileProcessor interfering with dags outside its ``processor_subdir``
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #33357:
URL: https://github.com/apache/airflow/pull/33357#discussion_r1293809443
##########
airflow/models/dag.py:
##########
@@ -3555,16 +3555,26 @@ def set_is_paused(self, is_paused: bool, including_subdags: bool = True, session
def deactivate_deleted_dags(
cls,
alive_dag_filelocs: Container[str],
+ processor_subdir: str | None = None,
Review Comment:
Good idea. I double checked but indeed this is almost never used, only here and in 1 test that I adjusted.
We can even narrow the type to just `str` as `get_dag_directory` guarantee a str value as input.
[Done](https://github.com/apache/airflow/pull/33357/commits/a6d11b4ac01127bf2dd789ccb5c5170fda3c9cae) :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on a diff in pull request #33357: Fix DagFileProcessor interfering with dags outside its ``processor_subdir``
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #33357:
URL: https://github.com/apache/airflow/pull/33357#discussion_r1293809443
##########
airflow/models/dag.py:
##########
@@ -3555,16 +3555,26 @@ def set_is_paused(self, is_paused: bool, including_subdags: bool = True, session
def deactivate_deleted_dags(
cls,
alive_dag_filelocs: Container[str],
+ processor_subdir: str | None = None,
Review Comment:
Good idea. I double checked but indeed this is almost never used, only here and in 1 test that I adjusted.
We can even narrow the type to just `str` as `get_dag_directory` guarantee a str value as input.
Done :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #33357: Fix DagFileProcessor interfering with dags outside its ``processor_subdir``
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #33357:
URL: https://github.com/apache/airflow/pull/33357#discussion_r1292955716
##########
airflow/models/dag.py:
##########
@@ -3555,16 +3555,26 @@ def set_is_paused(self, is_paused: bool, including_subdags: bool = True, session
def deactivate_deleted_dags(
cls,
alive_dag_filelocs: Container[str],
+ processor_subdir: str | None = None,
Review Comment:
Same goes for DagCode I guess.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on pull request #33357: Fix DagFileProcessor interfering with dags outside its ``processor_subdir``
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #33357:
URL: https://github.com/apache/airflow/pull/33357#issuecomment-1678034553
Comment was adressed, merging
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #33357: Fix DagFileProcessor interfering with dags outside its ``processor_subdir``
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #33357:
URL: https://github.com/apache/airflow/pull/33357#discussion_r1292952610
##########
airflow/models/dag.py:
##########
@@ -3555,16 +3555,26 @@ def set_is_paused(self, is_paused: bool, including_subdags: bool = True, session
def deactivate_deleted_dags(
cls,
alive_dag_filelocs: Container[str],
+ processor_subdir: str | None = None,
Review Comment:
Can we just make this a required argument? From what I can tell this is not used anywhere else, and although this is not explicitly marked as internal, I don’t think there’s reasonable use case to interact with DagModel directly.
##########
airflow/models/dag.py:
##########
@@ -3555,16 +3555,26 @@ def set_is_paused(self, is_paused: bool, including_subdags: bool = True, session
def deactivate_deleted_dags(
cls,
alive_dag_filelocs: Container[str],
+ processor_subdir: str | None = None,
Review Comment:
Can we just make this a required argument? From what I can tell this is not used anywhere else, and although this is not explicitly marked as internal, I don’t think there’s reasonable use case to interact with DagModel directly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun merged pull request #33357: Fix DagFileProcessor interfering with dags outside its ``processor_subdir``
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun merged PR #33357:
URL: https://github.com/apache/airflow/pull/33357
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org