potiuk merged PR #37937:
URL: https://github.com/apache/airflow/pull/37937
--
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.a
potiuk commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-2016043868
@ashb ?
--
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 unsubscr
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1535784673
##
airflow/models/baseoperator.py:
##
@@ -672,6 +709,21 @@ class derived from this one results in the creation of a
task object,
If set to `None` (default), t
potiuk commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1535760463
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,133 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreemen
potiuk commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1535756687
##
airflow/models/baseoperator.py:
##
@@ -672,6 +709,21 @@ class derived from this one results in the creation of a
task object,
If set to `None` (default),
potiuk commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1535749962
##
airflow/models/baseoperator.py:
##
@@ -1880,9 +1935,9 @@ def chain_linear(*elements: DependencyMixin |
Sequence[DependencyMixin]):
E.g.: suppose you want pr
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1535654117
##
airflow/models/baseoperator.py:
##
@@ -365,6 +373,62 @@ def partial(
)
+class ExecutorSafeguard:
+"""
+The ExecutorSafeguard decorator.
+
+Check
dabla commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-2012421572
> I've tried refactoring it with sentinel but now integration tests start
failing probably because it's considering the operator being called outside of
TaskInstance while this was not th
dabla commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-2012269421
I've tried refactoring it with sentinel but now integration tests start
failing probably because it's considering the operator being called outside of
TaskInstance
--
This is an automa
ashb commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-2009337199
> > ```python
> > __sentiel = object()
> > ```
>
> Just wondering where the sentiel object should be passed then to know it's
called from Airflow itself and not outside it? I u
ashb commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1531908083
##
airflow/models/baseoperator.py:
##
@@ -365,6 +373,62 @@ def partial(
)
+class ExecutorSafeguard:
+"""
+The ExecutorSafeguard decorator.
+
+Checks
potiuk commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1531890373
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,133 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreemen
dabla commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-2009273436
> ```python
> __sentiel = object()
> ```
Just wondering where the sentiel object should be passed then to know it's
called from Airflow itself and not outside it?
--
This i
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1531862517
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,133 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreement
ashb commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1531702097
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,133 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements
ashb commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-2009058981
How expensive is looking a traceback at runtime? I wonder if instead we
could do something like this:
```python
__sentiel = object()
class BaseOperator:
def execute(
ashb commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-2009033286
-1 (but not blocking) to _enforcement_ of this. I've expanded a bit on this
on the mailing list, just wanted to leave a comment here too
--
This is an automated message from the Apache
potiuk commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-2008819730
One static check to fix :).
--
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 c
dabla commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-2002366385
I still need to do one improvement and will try to improve the
warning/exception message and then I think it's ready.
--
This is an automated message from the Apache Git Service.
To res
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1526179389
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, patch,
dabla commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-1999477793
> > (as long as tests are fixed that is :) )
>
> That maybe good example when use `dag_maker` and move as db-tests is
better than use try to mock everything in sqlalchemy and TaskIn
potiuk commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-1996176245
>
Added allow_mixing parameter, I couldn't help it but documented it a bit
with example what will no more be allowed. That way it's well documented.
Looks good :)
--
This is
dabla commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-1995576706
Added allow_mixing parameter, I couldn't help it but documented it a bit
with example what will no more be allowed. That way it's well documented.
--
This is an automated message from
dabla commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-1995429773
> > Good idea. Another idea I suddenly thought of is that we could add an
Airflow config parameter which determines that, like we have the config
parameter for the unit_test_mode, than ha
dabla commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-1995254057
All tests passed now
--
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.
potiuk commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-1994940143
> Good idea. Another idea I suddenly thought of is that we could add an
Airflow config parameter which determines that, like we have the config
parameter for the unit_test_mode, than hav
dabla commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-1994878353
> > (as long as tests are fixed that is :) )
>
> That maybe good example when use `dag_maker` and move as db-tests is
better than use try to mock everything in sqlalchemy and TaskIn
dabla commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-1994873785
> > > (as long as tests are fixed that is :) )
> >
> >
> > I'm working on that I hope I will have them fix today.
> > But now my question remains, do we really throw an Except
potiuk commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-1994608453
> > (as long as tests are fixed that is :) )
>
> I'm working on that I hope I will have them fix today.
>
> But now my question remains, do we really throw an Exception when
Taragolis commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-1994562340
> (as long as tests are fixed that is :) )
That maybe good example when use `dag_maker` and move as db-tests is better
than use try to mock everything in sqlalchemy and TaskInst
dabla commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-1994447371
> (as long as tests are fixed that is :) )
I'm working on that I hope I will have them fix today.
But now my question remains, do we really throw an Exception when bad mixing
potiuk commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1523156231
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, patch,
potiuk commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-1994294008
(as long as tests are fixed that is :) )
--
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 t
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1520151157
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, patch,
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1520151157
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, patch,
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1516017667
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, patch,
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1516019510
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, patch,
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1516017667
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, patch,
Taragolis commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1515771397
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, pat
Taragolis commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1515736302
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, pat
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1515646723
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, patch,
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1515646723
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, patch,
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1515100619
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, patch,
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1514614748
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, patch,
Taragolis commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1514492716
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, pat
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1514482604
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, patch,
Taragolis commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1514470722
##
tests/models/test_baseoperatormeta.py:
##
@@ -0,0 +1,64 @@
+import os
+from typing import Any
+from unittest import TestCase
+from unittest.mock import Mock, pat
dabla commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-1980845822
> Is it possible (desirable?) to raise an error when the operator is
instantiated, instead of when `execute` is called?
I've tried that at first as that was also my first idea, but
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1514459241
##
airflow/models/baseoperator.py:
##
@@ -364,6 +366,23 @@ def partial(
)
+def executor_safeguard():
+def decorator(func):
+def wrapper(*args, **kw
dabla commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1514445180
##
airflow/models/baseoperator.py:
##
@@ -364,6 +366,23 @@ def partial(
)
+def executor_safeguard():
+def decorator(func):
+def wrapper(*args, **kw
uranusjr commented on PR #37937:
URL: https://github.com/apache/airflow/pull/37937#issuecomment-1980824303
Is it possible (desirable?) to raise an error when the operator is
instantiated, instead of when `execute` is called?
--
This is an automated message from the Apache Git Service.
To
Taragolis commented on code in PR #37937:
URL: https://github.com/apache/airflow/pull/37937#discussion_r1514413603
##
airflow/models/baseoperator.py:
##
@@ -364,6 +366,23 @@ def partial(
)
+def executor_safeguard():
+def decorator(func):
+def wrapper(*args,
dabla opened a new pull request, #37937:
URL: https://github.com/apache/airflow/pull/37937
…o include an execute safeguard which prohibits called the execute method
manually from a Python callable or @task decorated python method.
After the [discussion
](https://lists
53 matches
Mail list logo
