You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2023/09/01 13:10:25 UTC

[GitHub] [airflow] potiuk opened a new pull request, #34006: Explain the users how they can check if python code is top-level

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

   Many users have problem with it. Adding the way how they can check it easily.
   
   <!--
    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/
   -->
   
   
   
   <!-- 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.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


[GitHub] [airflow] potiuk merged pull request #34006: Explain the users how they can check if python code is top-level

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


-- 
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] eladkal commented on a diff in pull request #34006: Explain the users how they can check if python code is top-level

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


##########
docs/apache-airflow/best-practices.rst:
##########
@@ -176,6 +176,96 @@ Good example:
 
 In the Bad example, NumPy is imported each time the DAG file is parsed, which will result in suboptimal performance in the DAG file processing. In the Good example, NumPy is only imported when the task is running.
 
+Since it is not always obvious, see the next chapter to check how my code is "top-level" code.
+
+How to check if my code is "top-level" code
+-------------------------------------------
+
+In order to understand whether your code is "top-level" or not you need to understand a lot of
+intricacies of how parsing Python works. In general, when Python parses the python file it executes
+the code it sees, except (in general) internal code of the methods that it does not execute.
+
+It has a number of special cases that are not obvious - for example top-level code also means
+any code that is used to determine default values of methods.
+
+However, there is an easy way to check whether your code is "top-level" or not. You simply need to
+parse your code and see if the piece of code gets executed.
+
+Imagine this code:
+
+.. code-block:: python
+
+  from airflow import DAG
+  from airflow.operators.python import PythonOperator
+  import pendulum
+
+
+  def get_task_id():
+      return "print_array_task"  # <- is that code going to be executed?
+
+
+  def get_array():
+      return [1, 2, 3]  # <- is that code going to be executed?
+
+
+  with DAG(
+      dag_id="example_python_operator",
+      schedule=None,
+      start_date=pendulum.datetime(2021, 1, 1, tz="UTC"),
+      catchup=False,
+      tags=["example"],
+  ) as dag:
+
+      operator = PythonOperator(
+          task_id=get_task_id(),
+          python_callable=get_array,
+          dag=dag,

Review Comment:
   From what I gather from https://github.com/apache/airflow/discussions/33997 the confusion is more around templated parameters or parameters in general?
   
   I wonder if the author question was more around this case:
   
   ```
         operator = PythonOperator(
             task_id=get_task_id(),
             python_callable=get_array,
             op_kwargs={"0": my_func()}  # <- this one
             dag=dag,
   ```
   



-- 
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 a diff in pull request #34006: Explain the users how they can check if python code is top-level

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


##########
docs/apache-airflow/best-practices.rst:
##########
@@ -176,6 +176,96 @@ Good example:
 
 In the Bad example, NumPy is imported each time the DAG file is parsed, which will result in suboptimal performance in the DAG file processing. In the Good example, NumPy is only imported when the task is running.
 
+Since it is not always obvious, see the next chapter to check how my code is "top-level" code.
+
+How to check if my code is "top-level" code
+-------------------------------------------
+
+In order to understand whether your code is "top-level" or not you need to understand a lot of
+intricacies of how parsing Python works. In general, when Python parses the python file it executes
+the code it sees, except (in general) internal code of the methods that it does not execute.
+
+It has a number of special cases that are not obvious - for example top-level code also means
+any code that is used to determine default values of methods.
+
+However, there is an easy way to check whether your code is "top-level" or not. You simply need to
+parse your code and see if the piece of code gets executed.
+
+Imagine this code:
+
+.. code-block:: python
+
+  from airflow import DAG
+  from airflow.operators.python import PythonOperator
+  import pendulum
+
+
+  def get_task_id():
+      return "print_array_task"  # <- is that code going to be executed?
+
+
+  def get_array():
+      return [1, 2, 3]  # <- is that code going to be executed?
+
+
+  with DAG(
+      dag_id="example_python_operator",
+      schedule=None,
+      start_date=pendulum.datetime(2021, 1, 1, tz="UTC"),
+      catchup=False,
+      tags=["example"],
+  ) as dag:
+
+      operator = PythonOperator(
+          task_id=get_task_id(),
+          python_callable=get_array,
+          dag=dag,

Review Comment:
   Could be .. but the mechanism of checking it is the same. If they add `print` in my_func() they will also see  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