Re: [PR] Ensure that you can create a second DAG whilst another one is already "active" [airflow]
potiuk commented on code in PR #44484: URL: https://github.com/apache/airflow/pull/44484#discussion_r1863875060 ## task_sdk/tests/defintions/test_dag.py: ## @@ -417,3 +417,9 @@ def noop_pipeline(value): ... # Test that if arg is not passed it raises a type error as expected. with pytest.raises(TypeError): noop_pipeline() + +def test_create_dag_while_active_context(self): +"""Test that we can safely create a DAG whilst a DAG is activated via ``with dag1:``.""" +with DAG(dag_id="simple_dag"): +DAG(dag_id="dag2") +# No asserts needed, it just needs to not fail Review Comment: sure . That works too -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ensure that you can create a second DAG whilst another one is already "active" [airflow]
ashb merged PR #44484: URL: https://github.com/apache/airflow/pull/44484 -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ensure that you can create a second DAG whilst another one is already "active" [airflow]
ashb commented on code in PR #44484: URL: https://github.com/apache/airflow/pull/44484#discussion_r1863817485 ## task_sdk/tests/defintions/test_dag.py: ## @@ -417,3 +417,9 @@ def noop_pipeline(value): ... # Test that if arg is not passed it raises a type error as expected. with pytest.raises(TypeError): noop_pipeline() + +def test_create_dag_while_active_context(self): +"""Test that we can safely create a DAG whilst a DAG is activated via ``with dag1:``.""" +with DAG(dag_id="simple_dag"): +DAG(dag_id="dag2") +# No asserts needed, it just needs to not fail Review Comment: I value consistency more tbh. This did make me discover a couple of other yoda conditions we have in test. `assert True is ` style of things. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ensure that you can create a second DAG whilst another one is already "active" [airflow]
potiuk commented on code in PR #44484: URL: https://github.com/apache/airflow/pull/44484#discussion_r1863815322 ## task_sdk/tests/defintions/test_dag.py: ## @@ -417,3 +417,9 @@ def noop_pipeline(value): ... # Test that if arg is not passed it raises a type error as expected. with pytest.raises(TypeError): noop_pipeline() + +def test_create_dag_while_active_context(self): +"""Test that we can safely create a DAG whilst a DAG is activated via ``with dag1:``.""" +with DAG(dag_id="simple_dag"): +DAG(dag_id="dag2") +# No asserts needed, it just needs to not fail Review Comment: Why? At some point we might add a rule "flag tests without asserts" if that will be added in Ruff - which might be actually a useful suggestion for a new rule for them. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ensure that you can create a second DAG whilst another one is already "active" [airflow]
kaxil commented on code in PR #44484: URL: https://github.com/apache/airflow/pull/44484#discussion_r1863814440 ## task_sdk/tests/defintions/test_dag.py: ## @@ -417,3 +417,9 @@ def noop_pipeline(value): ... # Test that if arg is not passed it raises a type error as expected. with pytest.raises(TypeError): noop_pipeline() + +def test_create_dag_while_active_context(self): +"""Test that we can safely create a DAG whilst a DAG is activated via ``with dag1:``.""" +with DAG(dag_id="simple_dag"): +DAG(dag_id="dag2") +# No asserts needed, it just needs to not fail Review Comment: assert True is redundant :D -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ensure that you can create a second DAG whilst another one is already "active" [airflow]
ashb commented on code in PR #44484: URL: https://github.com/apache/airflow/pull/44484#discussion_r1863813546 ## task_sdk/tests/defintions/test_dag.py: ## @@ -417,3 +417,9 @@ def noop_pipeline(value): ... # Test that if arg is not passed it raises a type error as expected. with pytest.raises(TypeError): noop_pipeline() + +def test_create_dag_while_active_context(self): +"""Test that we can safely create a DAG whilst a DAG is activated via ``with dag1:``.""" +with DAG(dag_id="simple_dag"): +DAG(dag_id="dag2") +# No asserts needed, it just needs to not fail Review Comment: 🤷🏻 We have other tests that rely on things not throwing an exception, but nothing else I can see that does `assert True` -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ensure that you can create a second DAG whilst another one is already "active" [airflow]
potiuk commented on code in PR #44484: URL: https://github.com/apache/airflow/pull/44484#discussion_r1863812313 ## task_sdk/tests/defintions/test_dag.py: ## @@ -417,3 +417,9 @@ def noop_pipeline(value): ... # Test that if arg is not passed it raises a type error as expected. with pytest.raises(TypeError): noop_pipeline() + +def test_create_dag_while_active_context(self): +"""Test that we can safely create a DAG whilst a DAG is activated via ``with dag1:``.""" +with DAG(dag_id="simple_dag"): +DAG(dag_id="dag2") +# No asserts needed, it just needs to not fail Review Comment: Maybe :D ? -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ensure that you can create a second DAG whilst another one is already "active" [airflow]
potiuk commented on code in PR #44484: URL: https://github.com/apache/airflow/pull/44484#discussion_r1863812149 ## task_sdk/tests/defintions/test_dag.py: ## @@ -417,3 +417,9 @@ def noop_pipeline(value): ... # Test that if arg is not passed it raises a type error as expected. with pytest.raises(TypeError): noop_pipeline() + +def test_create_dag_while_active_context(self): +"""Test that we can safely create a DAG whilst a DAG is activated via ``with dag1:``.""" +with DAG(dag_id="simple_dag"): +DAG(dag_id="dag2") +# No asserts needed, it just needs to not fail Review Comment: ```suggestion assert True ``` -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ensure that you can create a second DAG whilst another one is already "active" [airflow]
potiuk commented on code in PR #44484: URL: https://github.com/apache/airflow/pull/44484#discussion_r1863812149 ## task_sdk/tests/defintions/test_dag.py: ## @@ -417,3 +417,9 @@ def noop_pipeline(value): ... # Test that if arg is not passed it raises a type error as expected. with pytest.raises(TypeError): noop_pipeline() + +def test_create_dag_while_active_context(self): +"""Test that we can safely create a DAG whilst a DAG is activated via ``with dag1:``.""" +with DAG(dag_id="simple_dag"): +DAG(dag_id="dag2") +# No asserts needed, it just needs to not fail Review Comment: ```suggestion assert True, "No asserts needed, it just needs to not fail" ``` -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ensure that you can create a second DAG whilst another one is already "active" [airflow]
ashb commented on code in PR #44484: URL: https://github.com/apache/airflow/pull/44484#discussion_r1863811336 ## task_sdk/tests/defintions/test_dag.py: ## @@ -417,3 +417,11 @@ def noop_pipeline(value): ... # Test that if arg is not passed it raises a type error as expected. with pytest.raises(TypeError): noop_pipeline() + +def test_create_dag_while_active_context(self): +""" +Test that we can safely create a DAG whilst a DAG is activated via ``with dag1:``. +""" Review Comment: ```suggestion """Test that we can safely create a DAG whilst a DAG is activated via ``with dag1:``.""" ``` -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ensure that you can create a second DAG whilst another one is already "active" [airflow]
ashb commented on code in PR #44484: URL: https://github.com/apache/airflow/pull/44484#discussion_r1863801387 ## task_sdk/tests/defintions/test_dag.py: ## @@ -417,3 +417,11 @@ def noop_pipeline(value): ... # Test that if arg is not passed it raises a type error as expected. with pytest.raises(TypeError): noop_pipeline() + +def test_create_dag_while_active_context(self): +""" +Test that we can safely create a DAG whilst a DAG is activated via ``with dag1:``. +""" Review Comment: ```suggestion """Test that we can safely create a DAG whilst a DAG is activated via ``with dag1:``.""" ``` -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
