You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/05/30 07:47:50 UTC

[GitHub] [airflow] hammerhead opened a new issue, #24014: Using `PostgresOperator`'s `params` parameter in dynamic task mapping results in error

hammerhead opened a new issue, #24014:
URL: https://github.com/apache/airflow/issues/24014

   ### Apache Airflow version
   
   2.3.1 (latest released)
   
   ### What happened
   
   Importing a DAG using `PostgresOperator` with `expand(params=[...])` fails, claiming `params` was already specified as a partial argument, even though it wasn't.
   
   ### What you think should happen instead
   
   The DAG imports successfully.
   
   ### How to reproduce
   
   ```python
   from pathlib import Path
   import pendulum
   from airflow.providers.postgres.operators.postgres import PostgresOperator
   from airflow.decorators import dag, task
   
   
   @dag(
       start_date=pendulum.datetime(2021, 11, 19, tz="UTC"),
       schedule_interval="@daily",
       catchup=False,
   )
   def test():
       query_values = [{"a": 1}, {"a": 2}]
   
       PostgresOperator.partial(
           task_id="simple_select",
           sql="SELECT {{ params.a }}",
       ).expand(params=query_values)
   
   test_dag = test()
   ```
   
   Exception during import:
   ```
   Broken DAG: [/usr/local/airflow/dags/test_dag.py] Traceback (most recent call last):
     File "/usr/local/lib/python3.9/site-packages/airflow/models/mappedoperator.py", line 199, in expand
       prevent_duplicates(self.kwargs, mapped_kwargs, fail_reason="mapping already partial")
     File "/usr/local/lib/python3.9/site-packages/airflow/models/mappedoperator.py", line 139, in prevent_duplicates
       raise TypeError(f"{fail_reason} argument: {duplicated_keys.pop()}")
   TypeError: mapping already partial argument: params
   ```
   
   ### Operating System
   
   macOS 12.4
   
   ### Versions of Apache Airflow Providers
   
   ```apache-airflow-providers-postgres==4.1.0```
   
   ### Deployment
   
   Astronomer
   
   ### Deployment details
   
   _No response_
   
   ### Anything else
   
   _No response_
   
   ### Are you willing to submit PR?
   
   - [ ] Yes I am willing to submit a PR!
   
   ### Code of Conduct
   
   - [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
   


-- 
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.apache.org

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


[GitHub] [airflow] uranusjr commented on issue #24014: Task mapping against 'params'

Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #24014:
URL: https://github.com/apache/airflow/issues/24014#issuecomment-1140869414

   Oh, actually we already mention this in the documentation:
   
   > Some arguments are not mappable and must be passed to `partial()`, such as `task_id`, `queue`, `pool`, and most other arguments to `BaseOperator`.
   
   https://airflow.apache.org/docs/apache-airflow/stable/concepts/dynamic-task-mapping.html


-- 
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] uranusjr closed issue #24014: Task mapping against 'params'

Posted by GitBox <gi...@apache.org>.
uranusjr closed issue #24014: Task mapping against 'params'
URL: https://github.com/apache/airflow/issues/24014


-- 
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] ashb commented on issue #24014: Task mapping against 'params'

Posted by GitBox <gi...@apache.org>.
ashb commented on issue #24014:
URL: https://github.com/apache/airflow/issues/24014#issuecomment-1143276455

   In this particular example the fix is to use `parameters` not `params` for the PostgresOperator https://airflow.apache.org/docs/apache-airflow-providers-postgres/stable/_api/airflow/providers/postgres/operators/postgres/index.html#airflow.providers.postgres.operators.postgres.PostgresOperator
   
   ![image](https://user-images.githubusercontent.com/34150/171360783-60ef4639-a40f-4bda-a4bd-b22b016b779c.png)
   


-- 
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] ashb commented on issue #24014: Task mapping against 'params'

Posted by GitBox <gi...@apache.org>.
ashb commented on issue #24014:
URL: https://github.com/apache/airflow/issues/24014#issuecomment-1195511357

   @uranusjr Could you add this to your todo list to fix please?


-- 
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] uranusjr commented on issue #24014: Using `PostgresOperator`'s `params` parameter in dynamic task mapping results in error

Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #24014:
URL: https://github.com/apache/airflow/issues/24014#issuecomment-1140841811

   PostgresOperator is misusing `params`. This argument should be 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


[GitHub] [airflow] hammerhead commented on issue #24014: Task mapping against 'params'

Posted by GitBox <gi...@apache.org>.
hammerhead commented on issue #24014:
URL: https://github.com/apache/airflow/issues/24014#issuecomment-1162819415

   Thanks, @josh-fell, the example you provided does work.
   However, I couldn't apply it successfully to our implementation, in which the value of `parameters` is provided by another task:
   ```python
       @task
       def parameter_values():
           # in our original implementation, this is a database query using pg_hook
           return [
               {"table_fqn": "raw_metrics", "column": "ts_day", "value": "v1"},
           ]
   
   
       sql = """
       DELETE FROM {{ task.mapped_kwargs.parameters[ti.map_index].table_fqn }}
       WHERE {{ task.mapped_kwargs.parameters[ti.map_index].column }} = {{ task.mapped_kwargs.parameters[ti.map_index].value }};"""
   
       PostgresOperator.partial(
           task_id="delete_partitions",
           postgres_conn_id="cratedb_connection",
           sql=sql,
       ).expand(parameters=parameter_values())
   ```
   
   This fails with an error related to XCom lookups:
   ```
   [2022-06-22, 07:54:34 UTC] {taskinstance.py:1889} ERROR - Task failed with exception
   Traceback (most recent call last):
     File "/usr/local/lib/python3.9/site-packages/airflow/models/taskinstance.py", line 1451, in _run_raw_task
       self._execute_task_with_callbacks(context, test_mode)
     File "/usr/local/lib/python3.9/site-packages/airflow/models/taskinstance.py", line 1555, in _execute_task_with_callbacks
       task_orig = self.render_templates(context=context)
     File "/usr/local/lib/python3.9/site-packages/airflow/models/taskinstance.py", line 2212, in render_templates
       rendered_task = self.task.render_template_fields(context)
     File "/usr/local/lib/python3.9/site-packages/airflow/models/mappedoperator.py", line 726, in render_template_fields
       self._do_render_template_fields(
     File "/usr/local/lib/python3.9/site-packages/airflow/utils/session.py", line 68, in wrapper
       return func(*args, **kwargs)
     File "/usr/local/lib/python3.9/site-packages/airflow/models/abstractoperator.py", line 344, in _do_render_template_fields
       rendered_content = self.render_template(
     File "/usr/local/lib/python3.9/site-packages/airflow/models/abstractoperator.py", line 391, in render_template
       return render_template_to_string(template, context)
     File "/usr/local/lib/python3.9/site-packages/airflow/utils/helpers.py", line 296, in render_template_to_string
       return render_template(template, context, native=False)
     File "/usr/local/lib/python3.9/site-packages/airflow/utils/helpers.py", line 291, in render_template
       return "".join(nodes)
     File "<template>", line 14, in root
     File "/usr/local/lib/python3.9/site-packages/jinja2/sandbox.py", line 303, in getitem
       return obj[argument]
     File "/usr/local/lib/python3.9/site-packages/airflow/models/xcom_arg.py", line 77, in __getitem__
       raise ValueError(f"XComArg only supports str lookup, received {type(item).__name__}")
   ValueError: XComArg only supports str lookup, received int
   ```


-- 
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 issue #24014: Task mapping against 'params'

Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #24014:
URL: https://github.com/apache/airflow/issues/24014#issuecomment-1140853666

   > Unfortunately `params` cannot be mapped at the time, since the value is currently needed in the scheduler, when the operator expansion is not resolved. We may change this in the future.
   
   Is there a way to raise exception that will explain to the user that the chosen parameter for expand is not supported?


-- 
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] hammerhead commented on issue #24014: Task mapping against 'params'

Posted by GitBox <gi...@apache.org>.
hammerhead commented on issue #24014:
URL: https://github.com/apache/airflow/issues/24014#issuecomment-1143372021

   > In this particular example the fix is to use `parameters` not `params` for the PostgresOperator https://airflow.apache.org/docs/apache-airflow-providers-postgres/stable/_api/airflow/providers/postgres/operators/postgres/index.html#airflow.providers.postgres.operators.postgres.PostgresOperator
   
   Yes, indeed. The original DAG I ran into the problem with was using different types of queries, such as `DELETE FROM {table_fqn} WHERE {column} = {value};`.
   In that example, I can't use `parameters` for `{table_fqn}` as `%(table_fqn)s` will wrap the value in single quotes, resulting in an SQL syntax error. Using `params` gives me the plain value instead.
   
   See also this [Stack Overflow post](https://stackoverflow.com/questions/70228542/how-to-force-airflow-not-to-put-single-quotes-around-rendered-name-in-postgresop) from another user.
   
   Right now, I'm using a Python task as a workaround and then pass it directly to the `sql` paramer of the `PostgresOperator`:
   ```python
   @task
   def generate_sql(policy):
       return Path('include/data_retention_delete.sql') \
           .read_text(encoding="utf-8").format(table_fqn=policy[0],
                                               column=policy[1],
                                               value=policy[2],
                                              )
   
   @dag(
       start_date=pendulum.datetime(2021, 11, 19, tz="UTC"),
       schedule_interval="@daily",
       catchup=False,
   )
   def data_retention_delete():
       sql_statements = generate_sql.expand(policy=get_policies())
   
       PostgresOperator.partial(
           task_id="delete_partition",
           postgres_conn_id="cratedb_connection",
       ).expand(sql=sql_statements)
   ```


-- 
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] josh-fell commented on issue #24014: Task mapping against 'params'

Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #24014:
URL: https://github.com/apache/airflow/issues/24014#issuecomment-1152839870

   @hammerhead See the "Anything else" section of #24388, but you should be able to still map via `parameters` for the `DELETE` DML in a Jinja-templated SQL using `{{ task.mapped_kwargs.parameters[ti.map_index].<attribute>}}` as a workaround.
   
   Assuming the desired SQL `DELETE FROM {table_fqn} WHERE {column} = {value};`, you could try:
   
   ```python
   sql = """
   DELETE FROM {{ task.mapped_kwargs.parameters[ti.map_index].table_fqn }}
   WHERE {{ task.mapped_kwargs.parameters[ti.map_index].column }} = {{ task.mapped_kwargs.parameters[ti.map_index].value }};"""
   
   PostgresOperator.partial(
       task_id="delete_partitions",
       postgres_conn_id="createdb_connection",
       sql=sql,
   ).expand(parameters=[
       {"table_fqn": "tbl1", "column": "col1", "value": "val1"},
       {"table_fqn": "tbl2", "column": "col2", "value": "val3"},
   )
   ```
   Does this help?


-- 
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] uranusjr commented on issue #24014: Task mapping against 'params'

Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #24014:
URL: https://github.com/apache/airflow/issues/24014#issuecomment-1140863968

   It’s possible, but not particularly doable unfortunately, since it’s difficult to tell apart what the user puts in when a function is called. With a function like this:
   
   ```python
   def foo(a=None): pass
   ```
   
   We don’t have a good way to tell if user called `foo(a=None)` or `foo()`. I’m putting together a PR to make the message slightly more vague so it’s at least not misleading, and add some documentation to explain 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