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/02/13 21:37:17 UTC

[GitHub] [airflow] victorphoenix3 opened a new pull request #21551: Add ability to pass config parameters to postgres operator

victorphoenix3 opened a new pull request #21551:
URL: https://github.com/apache/airflow/pull/21551


   Enable user to set [postgres connection parameters](https://www.postgresql.org/docs/14/runtime-config-client.html#RUNTIME-CONFIG-CLIENT-STATEMENT) through the operator
   
   Resolves #21486 


-- 
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] victorphoenix3 commented on a change in pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
victorphoenix3 commented on a change in pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#discussion_r815292841



##########
File path: airflow/providers/postgres/operators/postgres.py
##########
@@ -60,10 +63,28 @@ def __init__(
         self.autocommit = autocommit
         self.parameters = parameters
         self.database = database
+        self.runtime_parameters = runtime_parameters
         self.hook: Optional[PostgresHook] = None
 
     def execute(self, context: 'Context'):
         self.hook = PostgresHook(postgres_conn_id=self.postgres_conn_id, schema=self.database)
-        self.hook.run(self.sql, self.autocommit, parameters=self.parameters)
+        if self.runtime_parameters:
+            final_sql = []
+            sql_param = {}
+            for param in self.runtime_parameters:
+                set_param_sql = f"SET {{}} TO %({param})s;"
+                dynamic_sql = SQL(set_param_sql).format(Identifier(f"{param}"))
+                final_sql.append(dynamic_sql)
+            for param, val in self.runtime_parameters.items():
+                sql_param.update({f"{param}": f"{val}"})
+            if self.parameters:
+                sql_param.update(self.parameters)
+            if isinstance(self.sql, str):
+                final_sql.append(SQL(self.sql))
+            else:
+                final_sql.extend(list(map(SQL, self.sql)))
+            self.hook.run(final_sql, self.autocommit, parameters=sql_param)
+        else:
+            self.hook.run(self.sql, self.autocommit, parameters=self.parameters)

Review comment:
       This will execute when no server config parameters are passed by the user. In this case, `parameters` will only contain the parameters of the original sql query, like `begin date`, `end date` for the following example: `"SELECT * FROM pet WHERE birth_date BETWEEN SYMMETRIC %(begin_date)s AND %(end_date)s"
   `




-- 
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 merged pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
eladkal merged pull request #21551:
URL: https://github.com/apache/airflow/pull/21551


   


-- 
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 a change in pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#discussion_r806664237



##########
File path: airflow/providers/postgres/example_dags/example_postgres.py
##########
@@ -70,8 +70,9 @@
             BETWEEN SYMMETRIC DATE '{{ params.begin_date }}' AND DATE '{{ params.end_date }}';
             """,
         params={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},

Review comment:
       This example is wrong (not your change, but oops)
   
   ```suggestion
           parameters={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},
   ```




-- 
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 pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#issuecomment-1072345191


   @ashb ping :)


-- 
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 a change in pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#discussion_r806664890



##########
File path: airflow/providers/postgres/operators/postgres.py
##########
@@ -60,10 +61,17 @@ def __init__(
         self.autocommit = autocommit
         self.parameters = parameters
         self.database = database
+        self.runtime_parameters = runtime_parameters
         self.hook: Optional[PostgresHook] = None
 
     def execute(self, context: 'Context'):
         self.hook = PostgresHook(postgres_conn_id=self.postgres_conn_id, schema=self.database)
+        if self.runtime_parameters:
+            set_param_sql = [f"SET {param} TO '{val}'; " for param, val in self.runtime_parameters.items()]

Review comment:
       This feels prone to SQL injection. We should use SQLAlchemy constructs here to ensure we are not.




-- 
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 a change in pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#discussion_r806696184



##########
File path: airflow/providers/postgres/example_dags/example_postgres.py
##########
@@ -70,8 +70,9 @@
             BETWEEN SYMMETRIC DATE '{{ params.begin_date }}' AND DATE '{{ params.end_date }}';
             """,
         params={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},

Review comment:
       Oh yeah, Better (if we want to keep it parameterized at all) would be to use parameters so that we don't risk SQLI:
   
   ```
               BETWEEN SYMMETRIC DATE :begin_date: AND DATE :end_date:';
               """,
           parameters={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},
   ```
   (untested)




-- 
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] victorphoenix3 commented on pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
victorphoenix3 commented on pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#issuecomment-1038960487


   @eladkal I verified the PR by running the system test and checking the executed sql queries after passing the runtime_parameter to the PostgresOperator. Can you please elaborate on where I should add more tests and how? (New contributor here)


-- 
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 change in pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#discussion_r807271260



##########
File path: airflow/providers/postgres/example_dags/example_postgres.py
##########
@@ -70,8 +70,9 @@
             BETWEEN SYMMETRIC DATE '{{ params.begin_date }}' AND DATE '{{ params.end_date }}';
             """,
         params={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},

Review comment:
       agree.
   @victorphoenix3 can you take care of it?
   I think this is also the first example in all of our docs that will show the usage of `parameters`




-- 
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] victorphoenix3 commented on pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
victorphoenix3 commented on pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#issuecomment-1039947310


   @eladkal I have updated the tests and documentation


-- 
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 a change in pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#discussion_r806696184



##########
File path: airflow/providers/postgres/example_dags/example_postgres.py
##########
@@ -70,8 +70,9 @@
             BETWEEN SYMMETRIC DATE '{{ params.begin_date }}' AND DATE '{{ params.end_date }}';
             """,
         params={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},

Review comment:
       Oh yeah, Better would be to use parameters so that we don't risk SQLI:
   
   ```
               BETWEEN SYMMETRIC DATE :begin_date: AND DATE :end_date:';
               """,
           parameters={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},
   ```
   (untested)




-- 
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 pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#issuecomment-1072345191


   @ashb ping :)


-- 
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 pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#issuecomment-1072355773


   None, lgtm


-- 
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 pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#issuecomment-1072503801


   Thanks @victorphoenix3 


-- 
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] victorphoenix3 commented on pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
victorphoenix3 commented on pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#issuecomment-1056674898


   @eladkal does the PR require any more changes?


-- 
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 pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#issuecomment-1039010179


   Test should be added to:
   https://github.com/apache/airflow/blob/main/tests/providers/postgres/operators/test_postgres.py
   
   In general system tests will run on example dags so when you update example dag it will be picked up (eventually)
   https://github.com/apache/airflow/blob/main/tests/providers/postgres/operators/test_postgres_system.py
   
   
   For docs you will need to update:
   https://github.com/apache/airflow/blob/main/docs/apache-airflow-providers-postgres/operators/postgres_operator_howto_guide.rst


-- 
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 a change in pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#discussion_r806664237



##########
File path: airflow/providers/postgres/example_dags/example_postgres.py
##########
@@ -70,8 +70,9 @@
             BETWEEN SYMMETRIC DATE '{{ params.begin_date }}' AND DATE '{{ params.end_date }}';
             """,
         params={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},

Review comment:
       This example is wrong (not your change, but oops)
   
   ```suggestion
           parameters={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},
   ```

##########
File path: airflow/providers/postgres/operators/postgres.py
##########
@@ -60,10 +61,17 @@ def __init__(
         self.autocommit = autocommit
         self.parameters = parameters
         self.database = database
+        self.runtime_parameters = runtime_parameters
         self.hook: Optional[PostgresHook] = None
 
     def execute(self, context: 'Context'):
         self.hook = PostgresHook(postgres_conn_id=self.postgres_conn_id, schema=self.database)
+        if self.runtime_parameters:
+            set_param_sql = [f"SET {param} TO '{val}'; " for param, val in self.runtime_parameters.items()]

Review comment:
       This feels prone to SQL injection. We should use SQLAlchemy constructs here to ensure we are not.

##########
File path: airflow/providers/postgres/example_dags/example_postgres.py
##########
@@ -70,8 +70,9 @@
             BETWEEN SYMMETRIC DATE '{{ params.begin_date }}' AND DATE '{{ params.end_date }}';
             """,
         params={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},

Review comment:
       Oh yeah, Better would be to use parameters so that we don't risk SQLI:
   
   ```
               BETWEEN SYMMETRIC DATE :begin_date: AND DATE :end_date:';
               """,
           parameters={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},
   ```
   (untested)

##########
File path: airflow/providers/postgres/example_dags/example_postgres.py
##########
@@ -70,8 +70,9 @@
             BETWEEN SYMMETRIC DATE '{{ params.begin_date }}' AND DATE '{{ params.end_date }}';
             """,
         params={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},

Review comment:
       Oh yeah, Better (if we want to keep it parameterized at all) would be to use parameters so that we don't risk SQLI:
   
   ```
               BETWEEN SYMMETRIC DATE :begin_date: AND DATE :end_date:';
               """,
           parameters={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},
   ```
   (untested)




-- 
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 pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#issuecomment-1038867762


   Can you update also docs/example_dag and add test?


-- 
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 pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#issuecomment-1072355773


   None, lgtm


-- 
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 change in pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#discussion_r814059119



##########
File path: airflow/providers/postgres/operators/postgres.py
##########
@@ -60,10 +63,28 @@ def __init__(
         self.autocommit = autocommit
         self.parameters = parameters
         self.database = database
+        self.runtime_parameters = runtime_parameters
         self.hook: Optional[PostgresHook] = None
 
     def execute(self, context: 'Context'):
         self.hook = PostgresHook(postgres_conn_id=self.postgres_conn_id, schema=self.database)
-        self.hook.run(self.sql, self.autocommit, parameters=self.parameters)
+        if self.runtime_parameters:
+            final_sql = []
+            sql_param = {}
+            for param in self.runtime_parameters:
+                set_param_sql = f"SET {{}} TO %({param})s;"
+                dynamic_sql = SQL(set_param_sql).format(Identifier(f"{param}"))
+                final_sql.append(dynamic_sql)
+            for param, val in self.runtime_parameters.items():
+                sql_param.update({f"{param}": f"{val}"})
+            if self.parameters:
+                sql_param.update(self.parameters)
+            if isinstance(self.sql, str):
+                final_sql.append(SQL(self.sql))
+            else:
+                final_sql.extend(list(map(SQL, self.sql)))
+            self.hook.run(final_sql, self.autocommit, parameters=sql_param)
+        else:
+            self.hook.run(self.sql, self.autocommit, parameters=self.parameters)

Review comment:
       Does `parameters=self.parameters` means anything here?




-- 
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] github-actions[bot] commented on pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#issuecomment-1059936291


   The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.


-- 
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 change in pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#discussion_r806680629



##########
File path: airflow/providers/postgres/example_dags/example_postgres.py
##########
@@ -70,8 +70,9 @@
             BETWEEN SYMMETRIC DATE '{{ params.begin_date }}' AND DATE '{{ params.end_date }}';
             """,
         params={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},

Review comment:
       I don't think it's wrong. The code references to params by `params.begin_date`,` params.end_date`
   `parameters` is to pass the assignment to sqlalchemy engine (which means you will not see the full rendered query in render tab)
   
   Though I would say that params are not really needed here as the dates are static... so they can be just placed in the SQL directly




-- 
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 change in pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#discussion_r806680629



##########
File path: airflow/providers/postgres/example_dags/example_postgres.py
##########
@@ -70,8 +70,9 @@
             BETWEEN SYMMETRIC DATE '{{ params.begin_date }}' AND DATE '{{ params.end_date }}';
             """,
         params={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},

Review comment:
       I don't think it's wrong. The code references to params by `params.begin_date`,` params.end_date`
   `parameters` is to pass the assignment to sqlalchemy engine (which means you will not see the full rendered query in render tab)




-- 
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] victorphoenix3 commented on pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
victorphoenix3 commented on pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#issuecomment-1039947310


   @eladkal I have updated the tests and documentation


-- 
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 change in pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#discussion_r806617382



##########
File path: docs/apache-airflow-providers-postgres/operators/postgres_operator_howto_guide.rst
##########
@@ -134,6 +134,22 @@ To find the owner of the pet called 'Lester':
       parameters={"begin_date": "2020-01-01", "end_date": "2020-12-31"},
   )
 
+Passing Connection Parameters into PostgresOperator
+----------------------------------------
+
+PostgresOperator provides the optional ``runtime_parameters`` attribute which makes it possible to set
+the `server configuration parameter values <https://www.postgresql.org/docs/14/runtime-config-client.html>`_ for the SQL request during runtime.

Review comment:
       ```suggestion
   the `server configuration parameter values <https://www.postgresql.org/docs/current/runtime-config-client.html>`_ for the SQL request during runtime.
   ```
   
   so we will always direct to the updated docs of postgres

##########
File path: docs/apache-airflow-providers-postgres/operators/postgres_operator_howto_guide.rst
##########
@@ -134,6 +134,22 @@ To find the owner of the pet called 'Lester':
       parameters={"begin_date": "2020-01-01", "end_date": "2020-12-31"},
   )
 
+Passing Connection Parameters into PostgresOperator
+----------------------------------------
+
+PostgresOperator provides the optional ``runtime_parameters`` attribute which makes it possible to set
+the `server configuration parameter values <https://www.postgresql.org/docs/14/runtime-config-client.html>`_ for the SQL request during runtime.
+
+.. code-block:: python
+
+  get_birth_date = PostgresOperator(
+      task_id="get_birth_date",
+      postgres_conn_id="postgres_default",
+      sql="SELECT * FROM pet WHERE birth_date BETWEEN SYMMETRIC %(begin_date)s AND %(end_date)s",
+      parameters={"begin_date": "2020-01-01", "end_date": "2020-12-31"},
+      runtime_parameters={"statement_timeout": "30ms"},
+  )

Review comment:
       We prefer code examples to be in examples dag and reference these examples from the rst file
   check:
   https://github.com/apache/airflow/blob/main/airflow/providers/postgres/example_dags/example_postgres.py
   
   You can simply add another operator to the example and reference to it in the rst file (just make sure the timeout you set is reasonable to we won't have failures if/when system test are actually running the query




-- 
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 change in pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#discussion_r806680629



##########
File path: airflow/providers/postgres/example_dags/example_postgres.py
##########
@@ -70,8 +70,9 @@
             BETWEEN SYMMETRIC DATE '{{ params.begin_date }}' AND DATE '{{ params.end_date }}';
             """,
         params={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},

Review comment:
       I don't think it's wrong. The code references to params by `params.begin_date`,` params.end_date`
   `parameters` is to pass the assignment to sqlalchemy engine (which means you will not see the full rendered query in render tab)
   
   Though I would say that params/parameters are not really needed here anyway as the dates are static... so they can be just placed in the SQL directly




-- 
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 change in pull request #21551: Add ability to pass config parameters to postgres operator

Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #21551:
URL: https://github.com/apache/airflow/pull/21551#discussion_r806617382



##########
File path: docs/apache-airflow-providers-postgres/operators/postgres_operator_howto_guide.rst
##########
@@ -134,6 +134,22 @@ To find the owner of the pet called 'Lester':
       parameters={"begin_date": "2020-01-01", "end_date": "2020-12-31"},
   )
 
+Passing Connection Parameters into PostgresOperator
+----------------------------------------
+
+PostgresOperator provides the optional ``runtime_parameters`` attribute which makes it possible to set
+the `server configuration parameter values <https://www.postgresql.org/docs/14/runtime-config-client.html>`_ for the SQL request during runtime.

Review comment:
       ```suggestion
   the `server configuration parameter values <https://www.postgresql.org/docs/current/runtime-config-client.html>`_ for the SQL request during runtime.
   ```
   
   so we will always direct to the updated docs of postgres

##########
File path: docs/apache-airflow-providers-postgres/operators/postgres_operator_howto_guide.rst
##########
@@ -134,6 +134,22 @@ To find the owner of the pet called 'Lester':
       parameters={"begin_date": "2020-01-01", "end_date": "2020-12-31"},
   )
 
+Passing Connection Parameters into PostgresOperator
+----------------------------------------
+
+PostgresOperator provides the optional ``runtime_parameters`` attribute which makes it possible to set
+the `server configuration parameter values <https://www.postgresql.org/docs/14/runtime-config-client.html>`_ for the SQL request during runtime.
+
+.. code-block:: python
+
+  get_birth_date = PostgresOperator(
+      task_id="get_birth_date",
+      postgres_conn_id="postgres_default",
+      sql="SELECT * FROM pet WHERE birth_date BETWEEN SYMMETRIC %(begin_date)s AND %(end_date)s",
+      parameters={"begin_date": "2020-01-01", "end_date": "2020-12-31"},
+      runtime_parameters={"statement_timeout": "30ms"},
+  )

Review comment:
       We prefer code examples to be in examples dag and reference these examples from the rst file
   check:
   https://github.com/apache/airflow/blob/main/airflow/providers/postgres/example_dags/example_postgres.py
   
   You can simply add another operator to the example and reference to it in the rst file (just make sure the timeout you set is reasonable to we won't have failures if/when system test are actually running the query

##########
File path: airflow/providers/postgres/example_dags/example_postgres.py
##########
@@ -70,8 +70,9 @@
             BETWEEN SYMMETRIC DATE '{{ params.begin_date }}' AND DATE '{{ params.end_date }}';
             """,
         params={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},

Review comment:
       I don't think it's wrong. The code references to params by `params.begin_date`,` params.end_date`
   `parameters` is to pass the assignment to sqlalchemy engine (which means you will not see the full rendered query in render tab)

##########
File path: airflow/providers/postgres/example_dags/example_postgres.py
##########
@@ -70,8 +70,9 @@
             BETWEEN SYMMETRIC DATE '{{ params.begin_date }}' AND DATE '{{ params.end_date }}';
             """,
         params={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},

Review comment:
       I don't think it's wrong. The code references to params by `params.begin_date`,` params.end_date`
   `parameters` is to pass the assignment to sqlalchemy engine (which means you will not see the full rendered query in render tab)
   
   Though I would say that params are not really needed here as the dates are static... so they can be just placed in the SQL directly

##########
File path: airflow/providers/postgres/example_dags/example_postgres.py
##########
@@ -70,8 +70,9 @@
             BETWEEN SYMMETRIC DATE '{{ params.begin_date }}' AND DATE '{{ params.end_date }}';
             """,
         params={'begin_date': '2020-01-01', 'end_date': '2020-12-31'},

Review comment:
       I don't think it's wrong. The code references to params by `params.begin_date`,` params.end_date`
   `parameters` is to pass the assignment to sqlalchemy engine (which means you will not see the full rendered query in render tab)
   
   Though I would say that params/parameters are not really needed here anyway as the dates are static... so they can be just placed in the SQL directly




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