You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "o-nikolas (via GitHub)" <gi...@apache.org> on 2023/03/03 19:02:51 UTC
[GitHub] [airflow] o-nikolas opened a new pull request, #29609: Decouple production executor warning in dags UI
o-nikolas opened a new pull request, #29609:
URL: https://github.com/apache/airflow/pull/29609
Previously the views code was hardcoded to look out for the SequentialExecutor and warn to not use it in production. This change makes that generalized to any non-production executor
<!--
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 an 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 changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+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 a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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 #29609: Decouple production executor warning in dags UI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29609:
URL: https://github.com/apache/airflow/pull/29609#issuecomment-1454918303
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] potiuk commented on pull request #29609: Decouple production executor warning in dags UI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29609:
URL: https://github.com/apache/airflow/pull/29609#issuecomment-1445121921
I think it needs rebase.
--
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] dimberman commented on pull request #29609: Decouple production executor warning in dags UI
Posted by "dimberman (via GitHub)" <gi...@apache.org>.
dimberman commented on PR #29609:
URL: https://github.com/apache/airflow/pull/29609#issuecomment-1436081501
@o-nikolas looks like there are a few failing tests. Once these are fixed would be glad to merge.
--
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] o-nikolas commented on pull request #29609: Decouple production executor warning in dags UI
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on PR #29609:
URL: https://github.com/apache/airflow/pull/29609#issuecomment-1449246403
Interesting development:
In this PR I add the notion of `is_production` to the executor interface, to represent whether or not an executor is meant to be used in production code (executors like `DebugExecutor` and `SequentialExecutor`, for example, are not intended for production use).
But the only usage was in the `/run` endpoint and associated `dags.html` and @bbovenzi has removed that endpoint entirely in #29706
So shall we keep this PR which adds an attribute to the Executor interface which is now not used? Or scrap it and wait until we find another use case for it? WDYT @potiuk & @dimberman?
--
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] o-nikolas commented on pull request #29609: Decouple production executor warning in dags UI
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on PR #29609:
URL: https://github.com/apache/airflow/pull/29609#issuecomment-1453982931
Sorry for the confusion, but this code is still needed after all. I thought things had changed in a way that they haven't. I'll update this PR today
--
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] o-nikolas commented on pull request #29609: Decouple production executor warning in dags UI
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on PR #29609:
URL: https://github.com/apache/airflow/pull/29609#issuecomment-1453892080
Thanks for the feedback! I'll scrap it for now :+1:
--
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 #29609: Decouple production executor warning in dags UI
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #29609:
URL: https://github.com/apache/airflow/pull/29609#issuecomment-1452912263
Scraping it sounds reasonable to me
--
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 #29609: Decouple production executor warning in dags UI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29609:
URL: https://github.com/apache/airflow/pull/29609#issuecomment-1453320336
Yep. Scrap it @o-nikolas
--
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] o-nikolas closed pull request #29609: Decouple production executor warning in dags UI
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas closed pull request #29609: Decouple production executor warning in dags UI
URL: https://github.com/apache/airflow/pull/29609
--
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 #29609: Decouple production executor warning in dags UI
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #29609:
URL: https://github.com/apache/airflow/pull/29609
--
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] o-nikolas commented on pull request #29609: Decouple production executor warning in dags UI
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on PR #29609:
URL: https://github.com/apache/airflow/pull/29609#issuecomment-1454099713
@potiuk @dimberman @uranusjr
Sorry again for the confusion, but this PR is ready again for review :pray:
--
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] o-nikolas commented on pull request #29609: Decouple production executor warning in dags UI
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on PR #29609:
URL: https://github.com/apache/airflow/pull/29609#issuecomment-1452761063
Thoughts on the above anyone?
--
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