kacpermuda commented on code in PR #50234:
URL: https://github.com/apache/airflow/pull/50234#discussion_r2077389064
##########
providers/openlineage/tests/system/openlineage/transport/variable.py:
##########
@@ -22,6 +22,8 @@
from openlineage.client.serde import Serde
from openlineage.client.transport import Config, Transport, get_default_factory
+# We use `airflow.models.variable.Variable` instead of `airflow.sdk.Variable`,
even for Airflow 3, as it also
+# works on scheduler. It may emit DeprecationWarning on workers, but it's
needed for DAG events to be emitted.
Review Comment:
It would have to be a bit messy check for `if
hasattr(sys.modules.get("airflow.sdk.execution_time.task_runner"),
"SUPERVISOR_COMMS"):` as already implemented in
`airflow.models.variable.Variable`:
.
The problem is that we can import `airflow.sdk.Variable` when on the
scheduler, but it can't perform it's job e.g., can't set variable. The current
implementation in `airflow.models.variable.Variable` is checking not only if
sdk is importable but also if we are in runtime environment (and uses the sdk
Variable underneath then), but it can emit warning when on worker because of
that.
So for now, if we want one Variable that works for both workers and
scheduler, we need to stick to this one or do this conditional import. I think
for now we can leave it as is, but if you have a strong opinion on this one,
let me know.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
