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 2021/12/17 01:33:10 UTC
[GitHub] [airflow] pingzh opened a new pull request #20361: Add context var hook to inject more env vars
pingzh opened a new pull request #20361:
URL: https://github.com/apache/airflow/pull/20361
This is useful when customized env vars are needed, for e.g. airflow
cluster name.
cluster name env var is very needed when you have many clusters.
We also use this hook to inject the cost tagging info so that the underlying compute engine process can get this information.
<!--
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/
-->
---
**^ 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] pingzh edited a comment on pull request #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
pingzh edited a comment on pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#issuecomment-1005433855
@potiuk @uranusjr any ideas about this error? https://github.com/apache/airflow/runs/4711391810?check_suite_focus=true#step:8:8255
I have rebased the latest `main` branch.
--
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 #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
pingzh commented on pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#issuecomment-997251536
@kaxil @potiuk ah, the bot label it as the document. it is actually a new feature with a doc.
--
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 a change in pull request #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#discussion_r774981167
##########
File path: airflow/utils/operator_helpers.py
##########
@@ -75,6 +86,16 @@ def context_to_airflow_vars(context: Mapping[str, Any], in_env_var_format: bool
(dag_run, 'run_id', 'AIRFLOW_CONTEXT_DAG_RUN_ID'),
]
+ context_params = settings.get_airflow_context_vars(context)
+ for key, value in context_params.items():
+ if in_env_var_format:
+ if not key.startswith(ENV_VAR_FORMAT_PREFIX):
+ key = ENV_VAR_FORMAT_PREFIX + key.upper()
+ else:
+ if not key.startswith(DEFAULT_FORMAT_PREFIX):
+ key = DEFAULT_FORMAT_PREFIX + key
+ params[key] = value
Review comment:
Some validation is needed to ensure the dict still only contain str.
--
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 edited a comment on pull request #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
pingzh edited a comment on pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#issuecomment-1005433855
@potiuk @uranusjr any ideas about this error? https://github.com/apache/airflow/runs/4711391810?check_suite_focus=true#step:8:8255
I have rebased the latest `main` branch.
--
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 #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
pingzh commented on pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#issuecomment-1005433855
@potiuk @uranusjr any ideas about this error?
I have rebased the latest `main` branch.
--
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 #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#issuecomment-997273718
no problem. labels are just informative
--
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 a change in pull request #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#discussion_r778592427
##########
File path: tests/utils/test_operator_helpers.py
##########
@@ -71,6 +71,29 @@ def test_context_to_airflow_vars_all_context(self):
'AIRFLOW_CTX_DAG_EMAIL': 'email1@test.com',
}
+ def test_context_to_airflow_vars_with_default_context_vars(self):
+ with mock.patch('airflow.settings.get_airflow_context_vars') as mock_method:
+ airflow_cluster = 'cluster-a'
+ mock_method.return_value = {'airflow_cluster': airflow_cluster}
+
+ context_vars = operator_helpers.context_to_airflow_vars(self.context)
+ assert context_vars['airflow.ctx.airflow_cluster'] == airflow_cluster
+
+ context_vars = operator_helpers.context_to_airflow_vars(self.context, in_env_var_format=True)
+ assert context_vars['AIRFLOW_CTX_AIRFLOW_CLUSTER'] == airflow_cluster
+
+ with mock.patch('airflow.settings.get_airflow_context_vars') as mock_method:
+ mock_method.return_value = {'airflow_cluster': [1, 2]}
+ with pytest.raises(TypeError) as error:
+ assert "value of key <airflow_cluster> must be string, not <class 'list'>" == error.value
+ operator_helpers.context_to_airflow_vars(self.context)
Review comment:
```suggestion
with pytest.raises(TypeError) as error:
operator_helpers.context_to_airflow_vars(self.context)
assert "value of key <airflow_cluster> must be string, not <class 'list'>" == error.value
```
##########
File path: tests/utils/test_operator_helpers.py
##########
@@ -71,6 +71,29 @@ def test_context_to_airflow_vars_all_context(self):
'AIRFLOW_CTX_DAG_EMAIL': 'email1@test.com',
}
+ def test_context_to_airflow_vars_with_default_context_vars(self):
+ with mock.patch('airflow.settings.get_airflow_context_vars') as mock_method:
+ airflow_cluster = 'cluster-a'
+ mock_method.return_value = {'airflow_cluster': airflow_cluster}
+
+ context_vars = operator_helpers.context_to_airflow_vars(self.context)
+ assert context_vars['airflow.ctx.airflow_cluster'] == airflow_cluster
+
+ context_vars = operator_helpers.context_to_airflow_vars(self.context, in_env_var_format=True)
+ assert context_vars['AIRFLOW_CTX_AIRFLOW_CLUSTER'] == airflow_cluster
+
+ with mock.patch('airflow.settings.get_airflow_context_vars') as mock_method:
+ mock_method.return_value = {'airflow_cluster': [1, 2]}
+ with pytest.raises(TypeError) as error:
+ assert "value of key <airflow_cluster> must be string, not <class 'list'>" == error.value
+ operator_helpers.context_to_airflow_vars(self.context)
+
+ with mock.patch('airflow.settings.get_airflow_context_vars') as mock_method:
+ mock_method.return_value = {1: "value"}
+ with pytest.raises(TypeError) as error:
+ assert 'key <1> must be string' == error.value
+ operator_helpers.context_to_airflow_vars(self.context)
Review comment:
```suggestion
with pytest.raises(TypeError) as error:
operator_helpers.context_to_airflow_vars(self.context)
assert 'key <1> must be string' == error.value
```
--
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 #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#issuecomment-997274214
This one looks cool. But it touches some deep internals of Airflow so I would love to see others opinions if they see no dangers/problems with thiis approach.
--
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 #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
pingzh commented on a change in pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#discussion_r778598911
##########
File path: tests/utils/test_operator_helpers.py
##########
@@ -71,6 +71,29 @@ def test_context_to_airflow_vars_all_context(self):
'AIRFLOW_CTX_DAG_EMAIL': 'email1@test.com',
}
+ def test_context_to_airflow_vars_with_default_context_vars(self):
+ with mock.patch('airflow.settings.get_airflow_context_vars') as mock_method:
+ airflow_cluster = 'cluster-a'
+ mock_method.return_value = {'airflow_cluster': airflow_cluster}
+
+ context_vars = operator_helpers.context_to_airflow_vars(self.context)
+ assert context_vars['airflow.ctx.airflow_cluster'] == airflow_cluster
+
+ context_vars = operator_helpers.context_to_airflow_vars(self.context, in_env_var_format=True)
+ assert context_vars['AIRFLOW_CTX_AIRFLOW_CLUSTER'] == airflow_cluster
+
+ with mock.patch('airflow.settings.get_airflow_context_vars') as mock_method:
+ mock_method.return_value = {'airflow_cluster': [1, 2]}
+ with pytest.raises(TypeError) as error:
+ assert "value of key <airflow_cluster> must be string, not <class 'list'>" == error.value
+ operator_helpers.context_to_airflow_vars(self.context)
+
+ with mock.patch('airflow.settings.get_airflow_context_vars') as mock_method:
+ mock_method.return_value = {1: "value"}
+ with pytest.raises(TypeError) as error:
+ assert 'key <1> must be string' == error.value
+ operator_helpers.context_to_airflow_vars(self.context)
Review comment:
thanks, ah, my bad 🤦 , fixed.
--
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 #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
pingzh commented on a change in pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#discussion_r778486378
##########
File path: airflow/utils/operator_helpers.py
##########
@@ -75,6 +86,16 @@ def context_to_airflow_vars(context: Mapping[str, Any], in_env_var_format: bool
(dag_run, 'run_id', 'AIRFLOW_CONTEXT_DAG_RUN_ID'),
]
+ context_params = settings.get_airflow_context_vars(context)
+ for key, value in context_params.items():
+ if in_env_var_format:
+ if not key.startswith(ENV_VAR_FORMAT_PREFIX):
+ key = ENV_VAR_FORMAT_PREFIX + key.upper()
+ else:
+ if not key.startswith(DEFAULT_FORMAT_PREFIX):
+ key = DEFAULT_FORMAT_PREFIX + key
+ params[key] = value
Review comment:
good point. @potiuk @uranusjr addressed the comment.
--
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 #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
pingzh commented on a change in pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#discussion_r778558416
##########
File path: airflow/utils/operator_helpers.py
##########
@@ -77,6 +87,21 @@ def context_to_airflow_vars(context: Mapping[str, Any], in_env_var_format: bool
(dag_run, 'run_id', 'AIRFLOW_CONTEXT_DAG_RUN_ID'),
]
+ context_params = settings.get_airflow_context_vars(context)
+ for key, value in context_params.items():
+ if not isinstance(key, str):
+ raise TypeError("key must be string")
+ if not isinstance(value, str):
+ raise TypeError("value must be string")
Review comment:
i think this should be clear enough. but it wouldn't hurt to be more explicit.
--
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 pull request #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#issuecomment-1005438826
Oh, the last one is from your change though; you’re using `with pytest.raises(TypeError) as error` incorrectly.
--
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 removed a comment on pull request #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
pingzh removed a comment on pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#issuecomment-997000340
hmm, very strange about this check flake8 check. it passed in my local
but failed in ci
--
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 pull request #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#issuecomment-1006263362
Unfortunately it’s not possible to just rerun one job, only the entire CI (which is wasteful). Don’t worry about it, other jobs are passing.
--
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 #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#issuecomment-997274568
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, 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] uranusjr commented on a change in pull request #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#discussion_r778521753
##########
File path: airflow/utils/operator_helpers.py
##########
@@ -75,6 +86,16 @@ def context_to_airflow_vars(context: Mapping[str, Any], in_env_var_format: bool
(dag_run, 'run_id', 'AIRFLOW_CONTEXT_DAG_RUN_ID'),
]
+ context_params = settings.get_airflow_context_vars(context)
+ for key, value in context_params.items():
+ if in_env_var_format:
+ if not key.startswith(ENV_VAR_FORMAT_PREFIX):
+ key = ENV_VAR_FORMAT_PREFIX + key.upper()
+ else:
+ if not key.startswith(DEFAULT_FORMAT_PREFIX):
+ key = DEFAULT_FORMAT_PREFIX + key
+ params[key] = value
Review comment:
Should probably mention this in the documentation page 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 commented on pull request #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
pingzh commented on pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#issuecomment-1006262284
Hi @uranusjr , do you know a way to rerun a CI task? https://github.com/apache/airflow/runs/4721603153?check_suite_focus=true looks like this is not related to my changes and other CI tasks passed.
thanks
--
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 a change in pull request #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#discussion_r778521582
##########
File path: airflow/utils/operator_helpers.py
##########
@@ -75,6 +86,19 @@ def context_to_airflow_vars(context: Mapping[str, Any], in_env_var_format: bool
(dag_run, 'run_id', 'AIRFLOW_CONTEXT_DAG_RUN_ID'),
]
+ context_params = settings.get_airflow_context_vars(context)
+ for key, value in context_params.items():
+ assert type(key) == str, "key must be string"
+ assert type(value) == str, "value must be string"
Review comment:
This should use `isinstance` instead IMO. There’s also a rule to not use asserts in the code base (although I personally disagree with) https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#don-t-use-asserts-outside-tests
--
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 #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
pingzh commented on a change in pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#discussion_r778523450
##########
File path: airflow/utils/operator_helpers.py
##########
@@ -75,6 +86,19 @@ def context_to_airflow_vars(context: Mapping[str, Any], in_env_var_format: bool
(dag_run, 'run_id', 'AIRFLOW_CONTEXT_DAG_RUN_ID'),
]
+ context_params = settings.get_airflow_context_vars(context)
+ for key, value in context_params.items():
+ assert type(key) == str, "key must be string"
+ assert type(value) == str, "value must be string"
Review comment:
ah, thanks for pointing out 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
[GitHub] [airflow] uranusjr commented on pull request #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#issuecomment-1005437824
Looks unrelated, probably someone merged some bad code to main. I’ll investigate separately.
--
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 #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#discussion_r776029198
##########
File path: airflow/utils/operator_helpers.py
##########
@@ -75,6 +86,16 @@ def context_to_airflow_vars(context: Mapping[str, Any], in_env_var_format: bool
(dag_run, 'run_id', 'AIRFLOW_CONTEXT_DAG_RUN_ID'),
]
+ context_params = settings.get_airflow_context_vars(context)
+ for key, value in context_params.items():
+ if in_env_var_format:
+ if not key.startswith(ENV_VAR_FORMAT_PREFIX):
+ key = ENV_VAR_FORMAT_PREFIX + key.upper()
+ else:
+ if not key.startswith(DEFAULT_FORMAT_PREFIX):
+ key = DEFAULT_FORMAT_PREFIX + key
+ params[key] = value
Review comment:
Agree. Sanity check here is a good thing.
--
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 #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
pingzh commented on pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#issuecomment-997000340
hmm, very strange about this check flake8 check. it passed in my local
but failed in ci
--
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 a change in pull request #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#discussion_r778554258
##########
File path: airflow/utils/operator_helpers.py
##########
@@ -77,6 +87,21 @@ def context_to_airflow_vars(context: Mapping[str, Any], in_env_var_format: bool
(dag_run, 'run_id', 'AIRFLOW_CONTEXT_DAG_RUN_ID'),
]
+ context_params = settings.get_airflow_context_vars(context)
+ for key, value in context_params.items():
+ if not isinstance(key, str):
+ raise TypeError("key must be string")
+ if not isinstance(value, str):
+ raise TypeError("value must be string")
Review comment:
Can we show the value of `key` and `value` in the exception message? This would help the user debug the problem. Something like `f"key must be string, not {key!r}"` would be enough (and the value variant should show both `key` and `value`).
--
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 #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
pingzh commented on pull request #20361:
URL: https://github.com/apache/airflow/pull/20361#issuecomment-1006264814
> Unfortunately it’s not possible to just rerun one job, only the entire CI (which is wasteful). Don’t worry about it, other jobs are passing.
Thanks
--
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 merged pull request #20361: Add context var hook to inject more env vars
Posted by GitBox <gi...@apache.org>.
uranusjr merged pull request #20361:
URL: https://github.com/apache/airflow/pull/20361
--
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