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 2020/01/21 13:57:39 UTC
[GitHub] [airflow] TomatoEgg opened a new pull request #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
TomatoEgg opened a new pull request #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
URL: https://github.com/apache/airflow/pull/7230
…pport dynamic parameters for kubernetes executor
For kubernetes executor, it will create a Pod based on executor_config before rendering the
template fields & running the task, therefore it won't work if a custom ioperator defines executor_config
as template field. It also requires the change in airflow to render it before the task is
triggered
---
Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
Make sure to mark the boxes below before creating PR: [x]
- [x] Description above provides context of the change
- [x] Commit message/PR title starts with `[AIRFLOW-NNNN]`. AIRFLOW-NNNN = JIRA ID<sup>*</sup>
- [x] Unit tests coverage for changes (not needed for documentation changes)
- [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [] Relevant documentation is updated including usage instructions.
- [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
<sup>*</sup> For document-only changes commit message can start with `[AIRFLOW-XXXX]`.
---
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/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] TomatoEgg commented on issue #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
Posted by GitBox <gi...@apache.org>.
TomatoEgg commented on issue #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
URL: https://github.com/apache/airflow/pull/7230#issuecomment-580916713
@nuclearpinguin : sure, but I would need to discuss what I observed in my manual testing of this feature. Who is the best person to talk to? and how do you normally carry out such discussions? (This is my first PR so please bear with me those basic questions)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] stale[bot] closed pull request #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
URL: https://github.com/apache/airflow/pull/7230
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] boring-cyborg[bot] commented on issue #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
URL: https://github.com/apache/airflow/pull/7230#issuecomment-576693279
Congratulations on your first Pull Request and welcome to the Apache Airflow community!
If you have any issues or are unsure about any anything please check our
Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, pylint and type annotations). Our [pre-commits](
https://github.com/apache/airflow/blob/master/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks)
will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory).
Adding a new operator? Check this short [guide](https://github
.com/apache/airflow/blob/master/docs/howto/custom-operator.rst) Consider adding an example DAG that shows
how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/master/BREEZE.rst) for testing
locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from
Committers.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://apache-airflow-slack.herokuapp.com/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] TomatoEgg commented on issue #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
Posted by GitBox <gi...@apache.org>.
TomatoEgg commented on issue #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
URL: https://github.com/apache/airflow/pull/7230#issuecomment-581299939
@nuclearpinguin : basically I found out in my test that the following happens when I triggered a DAG with executor_config is templated (say {{ ds }} as value for a key):
1. scheduler_job._enqueue_task_instances_with_queued_state is called which invokes
```
self.executor.queue_command(
simple_task_instance,
command,
priority=priority,
queue=queue)
```
2. base_executor.queue_command() is called
```
self.queued_tasks[simple_task_instance.key] = (command, priority, queue, simple_task_instance)
```
In the above two steps, simple_task_instance is of type SimpleTaskInstance, which means that it doesn't have enough information to render the templates. If it is TaskInstance instead, we could render the executor_config in those places.
What makes this a bit more confusing is that the test case called 'test_enqueue_task_instances_with_queued_state' in test_scheduler_job.py, it is using TaskInstance as test data instead of SimpleTaskInstance.
So my question is
1) What data type is the correct one?
2) is there an integration test case that we could run to see the truth?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
URL: https://github.com/apache/airflow/pull/7230#issuecomment-581176490
I invite you to the channel #airflow-kubernetes on Slack.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on issue #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on issue #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
URL: https://github.com/apache/airflow/pull/7230#issuecomment-577130919
Travis is sad, can you fix pylint errors?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on issue #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on issue #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
URL: https://github.com/apache/airflow/pull/7230#issuecomment-581175777
> @nuclearpinguin : sure, but I would need to discuss what I observed in my manual testing of this feature. Who is the best person to talk to? and how do you normally carry out such discussions? (This is my first PR so please bear with me those basic questions)
We already have this PR so it is a good place to start a discussion. When there's no PR or the problem requires some more discussion (for example introduction of "fundamental" changes) we can use our devlist or #development on our slack:
> In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://apache-airflow-slack.herokuapp.com/
I am not sure what means "this feature" but in case it means Breeze / pre-commits, I summon @potiuk
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] stale[bot] commented on issue #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on issue #7230: [AIRFLOW-6602]: render executor_config before triggering tasks to su…
URL: https://github.com/apache/airflow/pull/7230#issuecomment-601077125
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services