You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "JDarDagran (via GitHub)" <gi...@apache.org> on 2024/02/26 20:49:29 UTC

[PR] openlineage: add `opt-in` option [airflow]

JDarDagran opened a new pull request, #37725:
URL: https://github.com/apache/airflow/pull/37725

   
   
   <!--
    Licensed to the Apache Software Foundation (ASF) under one
    or more contributor license agreements.  See the NOTICE file
    distributed with this work for additional information
    regarding copyright ownership.  The ASF licenses this file
    to you under the Apache License, Version 2.0 (the
    "License"); you may not use this file except in compliance
    with the License.  You may obtain a copy of the License at
   
      http://www.apache.org/licenses/LICENSE-2.0
   
    Unless required by applicable law or agreed to in writing,
    software distributed under the License is distributed on an
    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    KIND, either express or implied.  See the License for the
    specific language governing permissions and limitations
    under the License.
    -->
   
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of an existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   ### Problem statement
   Currently you can only disable OpenLineage for all DAGs and tasks or selectively disable by operator class. Users report lack of possibility to do so on DAG/task level instead while opting in.
   
   ### Proposed solution
   Add `opt_in` option as well as `enable/disable_lineage` methods that work both on DAG and task objects.
   Internally it passes constant parameter (Airflow's `airflow.models.Param`) that is next checked in OpenLineage listener.
   
   
   <!-- Please keep an empty line above the dashes. -->
   ---
   **^ Add meaningful description above**
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
   In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
   


-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1504068148


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:
+        description: |
+          Set this to true if you want to enable OpenLineage metadata extraction and emission
+          on DAG and/or Task level.
+        type: boolean
+        default: "False"
+        example: ~
+        version_added: 1.6.0

Review Comment:
   Should I leave it empty? When does this option need to be filled out for the provider release?



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1538861208


##########
airflow/providers/openlineage/utils/utils.py:
##########
@@ -73,6 +78,16 @@ def is_operator_disabled(operator: BaseOperator | MappedOperator) -> bool:
     return get_fully_qualified_class_name(operator) in conf.disabled_operators()
 
 
+def is_selective_lineage_enabled(obj) -> bool:
+    """If selective enable is active check if DAG or Task is enabled to emit events."""
+    if not conf.selective_enable():
+        return True
+    if isinstance(obj, DAG):
+        return is_dag_lineage_enabled(obj)
+    else:
+        return is_task_lineage_enabled(obj)

Review Comment:
   Let’s add a guard so this is only called on operator objects. The error message would be confusing if this function is (incorrectly) used on a foreign object in the current implementation.



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1538855386


##########
airflow/providers/openlineage/utils/utils.py:
##########
@@ -73,6 +78,16 @@ def is_operator_disabled(operator: BaseOperator | MappedOperator) -> bool:
     return get_fully_qualified_class_name(operator) in conf.disabled_operators()
 
 
+def is_selective_lineage_enabled(obj) -> bool:
+    """If selective enable is active check if DAG or Task is enabled to emit events."""
+    if not conf.selective_enable():
+        return True

Review Comment:
   Is this correct? This function seems to return True when lineage is enabled, but this branch returns True if `selective_enable` returns False.



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1538317937


##########
airflow/providers/openlineage/plugins/listener.py:
##########
@@ -51,6 +57,16 @@ def __init__(self):
         self.log = logging.getLogger(__name__)
         self.extractor_manager = ExtractorManager()
         self.adapter = OpenLineageAdapter()
+        self._selective_enable = conf.getboolean("openlineage", "selective_enable", fallback=False)
+

Review Comment:
   Moved logic to `conf.py` and related.



##########
tests/providers/openlineage/utils/test_selective_enable.py:
##########
@@ -0,0 +1,72 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from airflow.decorators import dag, task
+from airflow.models import DAG
+from airflow.operators.empty import EmptyOperator
+from airflow.providers.openlineage.utils.selective_enable import (
+    DISABLE_OL_PARAM,
+    ENABLE_OL_PARAM,
+    ENABLE_OL_PARAM_NAME,
+    disable_lineage,
+    enable_lineage,
+)
+
+
+class TestOpenLineageSelectiveEnable:

Review Comment:
   Added test case.



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1504123209


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:

Review Comment:
   Both `explicit_lineage_control` and  `selective_lineage_control` work for me. `force_per_dag_opt_in` limits scope per DAG (which isn't true, you can control per task too). `lineage_on_demand` sounds misleading - on what demand, e.g. is this lineage whenever you choose to emit it?



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "tatiana (via GitHub)" <gi...@apache.org>.
tatiana commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1519514121


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:

Review Comment:
   Along the lines of what @kacpermuda proposed, what about... `selective_enable` or `exclusive_enable`? it's already in the `openlineage` configuration section, so we probably do not need to add lineage to its name



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "mobuchowski (via GitHub)" <gi...@apache.org>.
mobuchowski commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1503988848


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:

Review Comment:
   `force_per_dag_opt_in`?



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1504070137


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:
+        description: |
+          Set this to true if you want to enable OpenLineage metadata extraction and emission
+          on DAG and/or Task level.
+        type: boolean
+        default: "False"
+        example: ~
+        version_added: 1.6.0

Review Comment:
   Oh, it marks from which Airflow version it's compatible with, not Airflow provider, isn't it?



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1535448401


##########
airflow/providers/openlineage/utils/selective_enable.py:
##########
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations
+
+from typing import TypeVar
+
+from airflow.models import DAG, Operator, Param
+from airflow.models.xcom_arg import XComArg
+
+ENABLE_OL_PARAM_NAME = "_enable_ol"
+ENABLE_OL_PARAM = Param(True, const=True)
+DISABLE_OL_PARAM = Param(False, const=False)
+T = TypeVar("T", bound="DAG | Operator")
+
+
+def enable_lineage(obj: T) -> T:

Review Comment:
   The `disabled` configuration is designed for simplicity and minimal overhead. It's intended for situations where a provider is installed but not used. Since enabling specific features while globally disabling the provider creates conflicting behavior, I would avoid introducing additional logging in this mode.



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "tatiana (via GitHub)" <gi...@apache.org>.
tatiana commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1534099758


##########
airflow/providers/openlineage/plugins/listener.py:
##########
@@ -51,6 +57,15 @@ def __init__(self):
         self.log = logging.getLogger(__name__)
         self.extractor_manager = ExtractorManager()
         self.adapter = OpenLineageAdapter()
+        self._selective_enable = conf.getboolean("openlineage", "selective_enable", fallback=False)
+
+    def _is_lineage_enabled(self, obj) -> bool:

Review Comment:
   ATM we have `_is_disabled` at `openlineage/plugins/openlineage.py`, which I understand will enable/disable the actual listener. Would it make sense either to rename this `_is_lineage_enabled` to `_is_selective_lineage_enabled` or to rename the one in `openlineage/plugins/openlineage.py` to `_is_openlineage_listener_enabled` - so it's straightforward to distinguish between 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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1538862026


##########
docs/apache-airflow-providers-openlineage/guides/user.rst:
##########
@@ -263,6 +264,61 @@ a string of semicolon separated Airflow Operators full import paths to ``extract
 
   AIRFLOW__OPENLINEAGE__EXTRACTORS='full.path.to.ExtractorClass;full.path.to.AnotherExtractorClass'
 
+Enabling OpenLineage on DAG/task level
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+One can selectively enable OpenLineage for specific DAGs and tasks by using the ``opt_in`` policy.
+To enable this policy, set the ``opt_in`` option to True in the [openlineage] section of your Airflow configuration file:
+
+.. code-block:: ini
+
+    [openlineage]
+    opt_in = True

Review Comment:
   This needs to be updated.



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "tatiana (via GitHub)" <gi...@apache.org>.
tatiana commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1517892095


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:

Review Comment:
   More precise:`disabled_unless_with_context` or `enable_with_context_only`?



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "tatiana (via GitHub)" <gi...@apache.org>.
tatiana commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1517880761


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:

Review Comment:
   What about `disabled_unless_decorated`?



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "tatiana (via GitHub)" <gi...@apache.org>.
tatiana commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1517880761


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:

Review Comment:
   What about `disable_unless_decorated`?



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "mobuchowski (via GitHub)" <gi...@apache.org>.
mobuchowski commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1517981734


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:
+        description: |
+          Set this to true if you want to enable OpenLineage metadata extraction and emission
+          on DAG and/or Task level.

Review Comment:
   ```
   If this setting is enabled, OpenLineage integration won't collect and emit metadata, unless you explicitely enable it per `DAG` or `Task` using  `enable_lineage` contextmanager.
   ```



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "tatiana (via GitHub)" <gi...@apache.org>.
tatiana commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1519514121


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:

Review Comment:
   Along the lines of what @kacpermuda proposed, what about... `selective_enable`? it's already in the `openlineage` configuration section, so we probably do not need to add lineage to its name



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1538934144


##########
airflow/providers/openlineage/conf.py:
##########
@@ -51,6 +51,11 @@ def disabled_operators() -> set[str]:
     return set(operator.strip() for operator in option.split(";") if operator.strip())
 
 
+@cache
+def selective_enable() -> bool:
+    return conf.getboolean("openlineage", "selective_enable", fallback=False)

Review Comment:
   We can use _CONFIG_SECTION as in all other functions 😄 



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1504062094


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:

Review Comment:
   1. `explicit_lineage_control`
   2. `selective_lineage_control`
   3. `lineage_on_demand`
    ?



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "tatiana (via GitHub)" <gi...@apache.org>.
tatiana commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1517976751


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:

Review Comment:
   Python context manager (`with`). This may be more explicit:
   `disabled_unless_with_context_manager`



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "tatiana (via GitHub)" <gi...@apache.org>.
tatiana commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1517880761


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:

Review Comment:
   What about `disabled_unless_decorated` or `enable_decorated_only`?



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "tatiana (via GitHub)" <gi...@apache.org>.
tatiana commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1534099758


##########
airflow/providers/openlineage/plugins/listener.py:
##########
@@ -51,6 +57,15 @@ def __init__(self):
         self.log = logging.getLogger(__name__)
         self.extractor_manager = ExtractorManager()
         self.adapter = OpenLineageAdapter()
+        self._selective_enable = conf.getboolean("openlineage", "selective_enable", fallback=False)
+
+    def _is_lineage_enabled(self, obj) -> bool:

Review Comment:
   ATM we have `_is_disabled` at `openlineage/plugins/openlineage.py`, which I understand will enable/disable the actual listener.
   
   Would it make sense either to rename this `_is_lineage_enabled` to `_is_selective_lineage_enabled` or to rename the one in `openlineage/plugins/openlineage.py` to `_is_openlineage_listener_enabled` - so it's straightforward to distinguish between 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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1538954635


##########
airflow/providers/openlineage/conf.py:
##########
@@ -51,6 +51,11 @@ def disabled_operators() -> set[str]:
     return set(operator.strip() for operator in option.split(";") if operator.strip())
 
 
+@cache
+def selective_enable() -> bool:
+    return conf.getboolean("openlineage", "selective_enable", fallback=False)

Review Comment:
   True. Changed.



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1538855386


##########
airflow/providers/openlineage/utils/utils.py:
##########
@@ -73,6 +78,16 @@ def is_operator_disabled(operator: BaseOperator | MappedOperator) -> bool:
     return get_fully_qualified_class_name(operator) in conf.disabled_operators()
 
 
+def is_selective_lineage_enabled(obj) -> bool:
+    """If selective enable is active check if DAG or Task is enabled to emit events."""
+    if not conf.selective_enable():
+        return True

Review Comment:
   Is this correct? This function seems to return True when lineage is enabled, but this branch returns True if `selective_enable` returns False.



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1537812684


##########
airflow/providers/openlineage/plugins/listener.py:
##########
@@ -51,6 +57,16 @@ def __init__(self):
         self.log = logging.getLogger(__name__)
         self.extractor_manager = ExtractorManager()
         self.adapter = OpenLineageAdapter()
+        self._selective_enable = conf.getboolean("openlineage", "selective_enable", fallback=False)
+

Review Comment:
   After you merge the current main that includes #38033, there will be no need to assign this as an attibute, as it can be added to conf.py and cached.



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1538912359


##########
airflow/providers/openlineage/utils/utils.py:
##########
@@ -73,6 +78,16 @@ def is_operator_disabled(operator: BaseOperator | MappedOperator) -> bool:
     return get_fully_qualified_class_name(operator) in conf.disabled_operators()
 
 
+def is_selective_lineage_enabled(obj) -> bool:
+    """If selective enable is active check if DAG or Task is enabled to emit events."""
+    if not conf.selective_enable():
+        return True
+    if isinstance(obj, DAG):
+        return is_dag_lineage_enabled(obj)
+    else:
+        return is_task_lineage_enabled(obj)

Review Comment:
   Added.



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "tatiana (via GitHub)" <gi...@apache.org>.
tatiana commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1517976751


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:

Review Comment:
   Python context manager (`with`). This may be more explicit:
   `disabled_unless_with_context_manager` or `disabled_unless_with_ol_context_manager`



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1504037856


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:
+        description: |
+          Set this to true if you want to enable OpenLineage metadata extraction and emission
+          on DAG and/or Task level.
+        type: boolean
+        default: "False"
+        example: ~
+        version_added: 1.6.0

Review Comment:
   This version number does not look right?



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1519550694


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:
+        description: |
+          Set this to true if you want to enable OpenLineage metadata extraction and emission
+          on DAG and/or Task level.

Review Comment:
   Changed as suggested.



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1538912119


##########
docs/apache-airflow-providers-openlineage/guides/user.rst:
##########
@@ -263,6 +264,61 @@ a string of semicolon separated Airflow Operators full import paths to ``extract
 
   AIRFLOW__OPENLINEAGE__EXTRACTORS='full.path.to.ExtractorClass;full.path.to.AnotherExtractorClass'
 
+Enabling OpenLineage on DAG/task level
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+One can selectively enable OpenLineage for specific DAGs and tasks by using the ``opt_in`` policy.
+To enable this policy, set the ``opt_in`` option to True in the [openlineage] section of your Airflow configuration file:
+
+.. code-block:: ini
+
+    [openlineage]
+    opt_in = True

Review Comment:
   Yes, it does! Thanks for pointing this out.
   Updated.



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "tatiana (via GitHub)" <gi...@apache.org>.
tatiana commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1534113809


##########
airflow/providers/openlineage/utils/selective_enable.py:
##########
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations
+
+from typing import TypeVar
+
+from airflow.models import DAG, Operator, Param
+from airflow.models.xcom_arg import XComArg
+
+ENABLE_OL_PARAM_NAME = "_enable_ol"
+ENABLE_OL_PARAM = Param(True, const=True)
+DISABLE_OL_PARAM = Param(False, const=False)
+T = TypeVar("T", bound="DAG | Operator")
+
+
+def enable_lineage(obj: T) -> T:
+    if isinstance(obj, XComArg):
+        enable_lineage(obj.operator)
+        return obj
+    # propagate param to tasks
+    if isinstance(obj, DAG):
+        for task in obj.task_dict.values():
+            enable_lineage(task)
+    obj.params[ENABLE_OL_PARAM_NAME] = ENABLE_OL_PARAM
+    return obj
+
+
+def disable_lineage(obj: T) -> T:

Review Comment:
   It may be worth to add some `INFO` or `DEBUG` log message to highlight we're skipping to send lineage events for this 



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "tatiana (via GitHub)" <gi...@apache.org>.
tatiana commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1534104809


##########
airflow/providers/openlineage/utils/selective_enable.py:
##########
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations
+
+from typing import TypeVar
+
+from airflow.models import DAG, Operator, Param
+from airflow.models.xcom_arg import XComArg
+
+ENABLE_OL_PARAM_NAME = "_enable_ol"
+ENABLE_OL_PARAM = Param(True, const=True)
+DISABLE_OL_PARAM = Param(False, const=False)
+T = TypeVar("T", bound="DAG | Operator")
+
+

Review Comment:
   It is probably worth adding a comment somewhere highlighting that these settings only take effect when the OpenLineage listener is enabled and adding some reference to how/where this is handled.



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1535471096


##########
airflow/providers/openlineage/utils/selective_enable.py:
##########
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations
+
+from typing import TypeVar
+
+from airflow.models import DAG, Operator, Param
+from airflow.models.xcom_arg import XComArg
+
+ENABLE_OL_PARAM_NAME = "_enable_ol"
+ENABLE_OL_PARAM = Param(True, const=True)
+DISABLE_OL_PARAM = Param(False, const=False)
+T = TypeVar("T", bound="DAG | Operator")
+
+
+def enable_lineage(obj: T) -> T:
+    if isinstance(obj, XComArg):
+        enable_lineage(obj.operator)
+        return obj
+    # propagate param to tasks
+    if isinstance(obj, DAG):
+        for task in obj.task_dict.values():
+            enable_lineage(task)
+    obj.params[ENABLE_OL_PARAM_NAME] = ENABLE_OL_PARAM
+    return obj
+
+
+def disable_lineage(obj: T) -> T:

Review Comment:
   Added such debug logs in `is_dag/task_lineage_enabled`.



##########
airflow/providers/openlineage/plugins/listener.py:
##########
@@ -51,6 +57,15 @@ def __init__(self):
         self.log = logging.getLogger(__name__)
         self.extractor_manager = ExtractorManager()
         self.adapter = OpenLineageAdapter()
+        self._selective_enable = conf.getboolean("openlineage", "selective_enable", fallback=False)
+
+    def _is_lineage_enabled(self, obj) -> bool:

Review Comment:
   Renamed.



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1535468703


##########
airflow/providers/openlineage/utils/selective_enable.py:
##########
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations
+
+from typing import TypeVar
+
+from airflow.models import DAG, Operator, Param
+from airflow.models.xcom_arg import XComArg
+
+ENABLE_OL_PARAM_NAME = "_enable_ol"
+ENABLE_OL_PARAM = Param(True, const=True)
+DISABLE_OL_PARAM = Param(False, const=False)
+T = TypeVar("T", bound="DAG | Operator")
+
+

Review Comment:
   That's user-oriented comment that should be and is located in documentation. Code comments are meant to be developer-oriented.



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1519550481


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:

Review Comment:
   I changed it to `selective_enable`. Thanks @tatiana for the suggestions!



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1537810497


##########
tests/providers/openlineage/utils/test_selective_enable.py:
##########
@@ -0,0 +1,72 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from airflow.decorators import dag, task
+from airflow.models import DAG
+from airflow.operators.empty import EmptyOperator
+from airflow.providers.openlineage.utils.selective_enable import (
+    DISABLE_OL_PARAM,
+    ENABLE_OL_PARAM,
+    ENABLE_OL_PARAM_NAME,
+    disable_lineage,
+    enable_lineage,
+)
+
+
+class TestOpenLineageSelectiveEnable:

Review Comment:
   I think we should also add some test when there are different settings for DAG and Task. F.e. when enabled on DAG level but disabled for one of tasks.



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "mobuchowski (via GitHub)" <gi...@apache.org>.
mobuchowski merged PR #37725:
URL: https://github.com/apache/airflow/pull/37725


-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "tatiana (via GitHub)" <gi...@apache.org>.
tatiana commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1534109411


##########
airflow/providers/openlineage/utils/selective_enable.py:
##########
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations
+
+from typing import TypeVar
+
+from airflow.models import DAG, Operator, Param
+from airflow.models.xcom_arg import XComArg
+
+ENABLE_OL_PARAM_NAME = "_enable_ol"
+ENABLE_OL_PARAM = Param(True, const=True)
+DISABLE_OL_PARAM = Param(False, const=False)
+T = TypeVar("T", bound="DAG | Operator")
+
+
+def enable_lineage(obj: T) -> T:

Review Comment:
   If the `disabled` configuration is set to True, would it make sense to warn the users via a log message that this will not work?



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1504081457


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:
+        description: |
+          Set this to true if you want to enable OpenLineage metadata extraction and emission
+          on DAG and/or Task level.
+        type: boolean
+        default: "False"
+        example: ~
+        version_added: 1.6.0

Review Comment:
   For `disabled_for_operators` it's set to `1.1.0`, so `1.6.0` lgtm if it's the next release. I think it's provider version, not Airflow (that, we specify in dependencies section)



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "mobuchowski (via GitHub)" <gi...@apache.org>.
mobuchowski commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1505708132


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:

Review Comment:
   I think the name should indicate what setting it to `true` or `false` will do. `selective control` and `explicit control` mean nothing tbh



-- 
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


Re: [PR] openlineage: add `opt-in` option [airflow]

Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #37725:
URL: https://github.com/apache/airflow/pull/37725#discussion_r1517942728


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -77,6 +77,14 @@ config:
         example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
         default: ""
         version_added: 1.1.0
+      opt_in:

Review Comment:
   What is the `context` then?



-- 
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