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/03/22 03:55:54 UTC
[GitHub] [airflow] pingzh opened a new pull request #22416: Pass X-Presto-Client-Info in presto hook
pingzh opened a new pull request #22416:
URL: https://github.com/apache/airflow/pull/22416
<!--
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 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/
-->
so that in the presto side, it can track back to an airflow task
---
**^ 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 change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+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 [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.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
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #22416: Pass X-Presto-Client-Info in presto hook
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #22416:
URL: https://github.com/apache/airflow/pull/22416#issuecomment-1077569142
Jus don't import it from there. or catch an import and provide a suitable default. I do not know the history of DEFAULT_FORMAT_PREFIX so cannot give a definite answer, but definitely if you add this import - you are relying on 2.3 behaviour, which we should not do.
According to https://github.com/apache/airflow/blob/main/README.md#support-for-providers all the providers until 21st of May should ne 2.1-compatible.
--
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] pingzh commented on a change in pull request #22416: Pass X-Presto-Client-Info in presto hook
Posted by GitBox <gi...@apache.org>.
pingzh commented on a change in pull request #22416:
URL: https://github.com/apache/airflow/pull/22416#discussion_r834650996
##########
File path: airflow/providers/presto/hooks/presto.py
##########
@@ -27,6 +28,33 @@
from airflow.configuration import conf
from airflow.hooks.dbapi import DbApiHook
from airflow.models import Connection
+from airflow.utils.operator_helpers import AIRFLOW_VAR_NAME_FORMAT_MAPPING
+
+try:
+ from airflow.utils.operator_helpers import DEFAULT_FORMAT_PREFIX
+except ImportError:
+ # This is from airflow.utils.operator_helpers,
+ # For the sake of provider backward compatibility, this is hardcoded if import fails
+ # https://github.com/apache/airflow/pull/22416#issuecomment-1075531290
+ DEFAULT_FORMAT_PREFIX = 'airflow.ctx.'
+
+
+def generate_presto_client_info() -> str:
+ """Return json string with dag_id, task_id, execution_date and try_number"""
+ context_var = {
+ format_map['default'].replace(DEFAULT_FORMAT_PREFIX, ''): os.environ.get(
+ format_map['env_var_format'], ''
+ )
+ for format_map in AIRFLOW_VAR_NAME_FORMAT_MAPPING.values()
+ }
+ task_info = {
+ 'dag_id': context_var['dag_id'],
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
[GitHub] [airflow] potiuk commented on pull request #22416: Pass X-Presto-Client-Info in presto hook
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22416:
URL: https://github.com/apache/airflow/pull/22416#issuecomment-1077569142
Jus don't import it from there. or catch an import and provide a suitable default. I do not know the history of DEFAULT_FORMAT_PREFIX so cannot give a definite answer, but definitely if you add this import - you are relying on 2.2 behaviour, which we should not do.
According to https://github.com/apache/airflow/blob/main/README.md#support-for-providers all the providers until 21st of May should ne 2.1-compatible.
--
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 pull request #22416: Pass X-Presto-Client-Info in presto hook
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22416:
URL: https://github.com/apache/airflow/pull/22416#issuecomment-1077571910
Seems that it's you who added the prefix, so you should know what to use in the provider to make it 2.1 comatible I think :)
--
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 #22416: Pass X-Presto-Client-Info in presto hook
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #22416:
URL: https://github.com/apache/airflow/pull/22416
--
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] pingzh commented on pull request #22416: Pass X-Presto-Client-Info in presto hook
Posted by GitBox <gi...@apache.org>.
pingzh commented on pull request #22416:
URL: https://github.com/apache/airflow/pull/22416#issuecomment-1078459727
> @pingzh Should we do that also for TrinoHook? https://trino.io/docs/current/develop/client-protocol.html#client-request-headers
@eladkal good catch. let me add that as well.
--
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 pull request #22416: Pass X-Presto-Client-Info in presto hook
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22416:
URL: https://github.com/apache/airflow/pull/22416#issuecomment-1077574795
Seems that the prefix was actually added in 2.3 so you MUST make it compatible because 2.2 compatibility will be there for quite a while. The 2.2 compatibility is there till October 11.
--
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 change in pull request #22416: Pass X-Presto-Client-Info in presto hook
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #22416:
URL: https://github.com/apache/airflow/pull/22416#discussion_r831882640
##########
File path: airflow/providers/presto/hooks/presto.py
##########
@@ -27,6 +28,24 @@
from airflow.configuration import conf
from airflow.hooks.dbapi import DbApiHook
from airflow.models import Connection
+from airflow.utils.operator_helpers import AIRFLOW_VAR_NAME_FORMAT_MAPPING, DEFAULT_FORMAT_PREFIX
+
+
+def generate_presto_client_info() -> str:
+ """Return json string with dag_id, task_id, execution_date and try_number"""
+ context_var = {
+ format_map['default'].replace(DEFAULT_FORMAT_PREFIX, ''): os.environ.get(
+ format_map['env_var_format'], ''
+ )
+ for format_map in AIRFLOW_VAR_NAME_FORMAT_MAPPING.values()
+ }
+ task_info = {
+ 'dag_id': context_var['dag_id'],
+ 'task_id': context_var['task_id'],
+ 'execution_date': context_var['execution_date'],
Review comment:
You should have rn _id here as well.
##########
File path: airflow/providers/presto/hooks/presto.py
##########
@@ -27,6 +28,24 @@
from airflow.configuration import conf
from airflow.hooks.dbapi import DbApiHook
from airflow.models import Connection
+from airflow.utils.operator_helpers import AIRFLOW_VAR_NAME_FORMAT_MAPPING, DEFAULT_FORMAT_PREFIX
+
+
+def generate_presto_client_info() -> str:
+ """Return json string with dag_id, task_id, execution_date and try_number"""
+ context_var = {
+ format_map['default'].replace(DEFAULT_FORMAT_PREFIX, ''): os.environ.get(
+ format_map['env_var_format'], ''
+ )
+ for format_map in AIRFLOW_VAR_NAME_FORMAT_MAPPING.values()
+ }
+ task_info = {
+ 'dag_id': context_var['dag_id'],
+ 'task_id': context_var['task_id'],
+ 'execution_date': context_var['execution_date'],
Review comment:
You should have ruin _id here as well.
--
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] pingzh closed pull request #22416: Pass X-Presto-Client-Info in presto hook
Posted by GitBox <gi...@apache.org>.
pingzh closed pull request #22416:
URL: https://github.com/apache/airflow/pull/22416
--
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 #22416: Pass X-Presto-Client-Info in presto hook
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #22416:
URL: https://github.com/apache/airflow/pull/22416#issuecomment-1078435834
@pingzh Should we do that also for TrinoHook?
https://trino.io/docs/current/develop/client-protocol.html#client-request-headers
--
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 edited a comment on pull request #22416: Pass X-Presto-Client-Info in presto hook
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #22416:
URL: https://github.com/apache/airflow/pull/22416#issuecomment-1077569142
Jus don't import it from there. or catch an import and provide a suitable default. I do not know the history of DEFAULT_FORMAT_PREFIX so cannot give a definite answer, but definitely if you add this import - you are relying on 2.3 behaviour, which we should not do.
According to https://github.com/apache/airflow/blob/main/README.md#support-for-providers all the providers until 21st of May should be 2.1-compatible.
--
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] pingzh closed pull request #22416: Pass X-Presto-Client-Info in presto hook
Posted by GitBox <gi...@apache.org>.
pingzh closed pull request #22416:
URL: https://github.com/apache/airflow/pull/22416
--
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 #22416: Pass X-Presto-Client-Info in presto hook
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22416:
URL: https://github.com/apache/airflow/pull/22416#discussion_r834522320
##########
File path: airflow/providers/presto/hooks/presto.py
##########
@@ -27,6 +28,33 @@
from airflow.configuration import conf
from airflow.hooks.dbapi import DbApiHook
from airflow.models import Connection
+from airflow.utils.operator_helpers import AIRFLOW_VAR_NAME_FORMAT_MAPPING
+
+try:
+ from airflow.utils.operator_helpers import DEFAULT_FORMAT_PREFIX
+except ImportError:
+ # This is from airflow.utils.operator_helpers,
+ # For the sake of provider backward compatibility, this is hardcoded if import fails
+ # https://github.com/apache/airflow/pull/22416#issuecomment-1075531290
+ DEFAULT_FORMAT_PREFIX = 'airflow.ctx.'
+
+
+def generate_presto_client_info() -> str:
+ """Return json string with dag_id, task_id, execution_date and try_number"""
+ context_var = {
+ format_map['default'].replace(DEFAULT_FORMAT_PREFIX, ''): os.environ.get(
+ format_map['env_var_format'], ''
+ )
+ for format_map in AIRFLOW_VAR_NAME_FORMAT_MAPPING.values()
+ }
+ task_info = {
+ 'dag_id': context_var['dag_id'],
Review comment:
Can we add also the dag owner?
--
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] pingzh commented on pull request #22416: Pass X-Presto-Client-Info in presto hook
Posted by GitBox <gi...@apache.org>.
pingzh commented on pull request #22416:
URL: https://github.com/apache/airflow/pull/22416#issuecomment-1075531290
@potiuk any suggestions that I can fix this https://github.com/apache/airflow/runs/5648830794?check_suite_focus=true#step:11:1039
since there is not `DEFAULT_FORMAT_PREFIX` in 2.1.1 https://github.com/apache/airflow/blob/2.1.1/airflow/utils/operator_helpers.py#L20
```
There were errors when importing Providers!
This test is run to check Provider's compatibility with Airflow 2.1.0
So this error might mean that your providers are not Airflow 2.1.0 compatible
Detailed errors:
----------------------------------------
Exception when importing: airflow.providers.google.cloud.example_dags.example_presto_to_gcs
Traceback (most recent call last):
File "/opt/airflow/dev/import_all_classes.py", line 90, in import_all_classes
_module = importlib.import_module(modinfo.name)
File "/usr/local/lib/python3.7/importlib/__init__.py", line 127, in import_module
return _bootstrap._gcd_import(name, package, level)
File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
File "<frozen importlib._bootstrap>", line 983, in _find_and_load
File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
File "<frozen importlib._bootstrap_external>", line 728, in exec_module
File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
File "/usr/local/lib/python3.7/site-packages/airflow/providers/google/cloud/example_dags/example_presto_to_gcs.py", line 32, in <module>
from airflow.providers.google.cloud.transfers.presto_to_gcs import PrestoToGCSOperator
File "/usr/local/lib/python3.7/site-packages/airflow/providers/google/cloud/transfers/presto_to_gcs.py", line 24, in <module>
from airflow.providers.presto.hooks.presto import PrestoHook
File "/usr/local/lib/python3.7/site-packages/airflow/providers/presto/hooks/presto.py", line 31, in <module>
from airflow.utils.operator_helpers import AIRFLOW_VAR_NAME_FORMAT_MAPPING, DEFAULT_FORMAT_PREFIX
ImportError: cannot import name 'DEFAULT_FORMAT_PREFIX' from 'airflow.utils.operator_helpers' (/usr/local/lib/python3.7/site-packages/airflow/utils/operator_helpers.py)
```
--
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] pingzh commented on a change in pull request #22416: Pass X-Presto-Client-Info in presto hook
Posted by GitBox <gi...@apache.org>.
pingzh commented on a change in pull request #22416:
URL: https://github.com/apache/airflow/pull/22416#discussion_r832422572
##########
File path: airflow/providers/presto/hooks/presto.py
##########
@@ -27,6 +28,24 @@
from airflow.configuration import conf
from airflow.hooks.dbapi import DbApiHook
from airflow.models import Connection
+from airflow.utils.operator_helpers import AIRFLOW_VAR_NAME_FORMAT_MAPPING, DEFAULT_FORMAT_PREFIX
+
+
+def generate_presto_client_info() -> str:
+ """Return json string with dag_id, task_id, execution_date and try_number"""
+ context_var = {
+ format_map['default'].replace(DEFAULT_FORMAT_PREFIX, ''): os.environ.get(
+ format_map['env_var_format'], ''
+ )
+ for format_map in AIRFLOW_VAR_NAME_FORMAT_MAPPING.values()
+ }
+ task_info = {
+ 'dag_id': context_var['dag_id'],
+ 'task_id': context_var['task_id'],
+ 'execution_date': context_var['execution_date'],
Review comment:
good idea. 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