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/07/21 13:51:09 UTC
[GitHub] [airflow] lwyszomi opened a new pull request #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
lwyszomi opened a new pull request #17137:
URL: https://github.com/apache/airflow/pull/17137
Related: https://github.com/apache/airflow/issues/13779
Give a possibility to run `wait_for_pipeline_state` with default or provided parameters, remove unnecessary call inside `start_pipeline` hook.
---
**^ 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] uranusjr closed pull request #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
uranusjr closed pull request #17137:
URL: https://github.com/apache/airflow/pull/17137
--
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] boring-cyborg[bot] commented on pull request #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#issuecomment-884206185
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/main/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/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/main/docs/apache-airflow/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/main/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.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
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://s.apache.org/airflow-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.
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 #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #17137:
URL: https://github.com/apache/airflow/pull/17137
--
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] lwyszomi commented on pull request #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#issuecomment-895187875
Hi @potiuk, I tryied to understand what's happen with build, but no ideas why these builds failed. I downloaded them locally run tests and all passed. Could you explain me a little what's happen? These failing builds should not be related to my 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] lwyszomi commented on pull request #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#issuecomment-901123029
@uranusjr both checks failed because the timeouts.
--
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 closed pull request #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
uranusjr closed pull request #17137:
URL: https://github.com/apache/airflow/pull/17137
--
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] lwyszomi commented on pull request #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#issuecomment-901123029
@uranusjr both checks failed because the timeouts.
--
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 closed pull request #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
uranusjr closed pull request #17137:
URL: https://github.com/apache/airflow/pull/17137
--
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] lwyszomi commented on a change in pull request #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#discussion_r686043241
##########
File path: tests/providers/google/cloud/hooks/test_datafusion.py
##########
@@ -197,13 +196,6 @@ def test_start_pipeline(self, mock_wait_for_pipeline_state, mock_request, hook):
mock_request.assert_called_once_with(
url=f"{INSTANCE_URL}/v3/namespaces/default/start", method="POST", body=body
)
- mock_wait_for_pipeline_state.assert_called_once_with(
- instance_url=INSTANCE_URL,
- namespace="default",
- pipeline_name=PIPELINE_NAME,
- pipeline_id=run_id,
- success_states=SUCCESS_STATES + [PipelineStates.RUNNING],
- )
Review comment:
Test extended @uranusjr
--
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 #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#issuecomment-896069448
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] lwyszomi commented on pull request #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#issuecomment-895770385
@potiuk Thank you for your response. I know that feeling when in big project we have some parts which randomly failing. I will try to what I can to help community to improve this process.
--
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 #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#discussion_r685778883
##########
File path: tests/providers/google/cloud/hooks/test_datafusion.py
##########
@@ -197,13 +196,6 @@ def test_start_pipeline(self, mock_wait_for_pipeline_state, mock_request, hook):
mock_request.assert_called_once_with(
url=f"{INSTANCE_URL}/v3/namespaces/default/start", method="POST", body=body
)
- mock_wait_for_pipeline_state.assert_called_once_with(
- instance_url=INSTANCE_URL,
- namespace="default",
- pipeline_name=PIPELINE_NAME,
- pipeline_id=run_id,
- success_states=SUCCESS_STATES + [PipelineStates.RUNNING],
- )
Review comment:
Since we removed this, should a case be added on the operator tests to make sure this is called?
--
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] lwyszomi commented on pull request #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#issuecomment-899426550
@uranusjr Thank you
--
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] lwyszomi commented on pull request #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#issuecomment-895802566
@potiuk I rebased PR, I hope this help :) sorry that I ping you when you are on the vacation.
--
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] lwyszomi commented on pull request #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#issuecomment-902653743
@uranusjr finally is green π
--
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] lwyszomi commented on pull request #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#issuecomment-899419712
@potiuk @uranusjr Can we trigger build maybe it will pass π
--
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 #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#issuecomment-895776838
@lwyszomi - actually your comment yesterday made me take a closer look to some of the failing cases (I am no vacations officialy but well, it's fun :) ) and I might have found a reason for big class of the most annoying issues (those that leave no traces behind and you are there with failed runner without any logs).
It's just merged **now** https://github.com/apache/airflow/pull/17524 so if you can rebase your PR again, there is a chance your builds will not fail for **that** reason at least. There are some more flakes we are independently looking at but I have high hopes that at least the most annoying one will be gone.
--
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 #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#issuecomment-899424131
Itβs stuck somehow.
--
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 #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#issuecomment-895313998
True they should not. But they do (and more often than we want) we are.continuously trying to prove flakes and false negatives ... But seems those GitHub runners became less stable recently. Usually committers will see it and merge those that look like intermittent - but we need to spend some time and try to see why it is happening (we continue solving those issues but new ones are continuously added - this is a constant struggle which currently we are a little on 'loosing' side - but we will keep on fighting. I am going to take a look at those soon - after my vacations and try to find out what could be improved and how to decrease the amount of false negatives. Seems this is somehow with some tests sometimes spinning out of control with memory and killing the tests, sometimes those are particular tests that are flakey (and we try to quarantine and fix them). Any suggestions and PRs improving those are welcome.
--
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] boring-cyborg[bot] commented on pull request #17137: [Airflow 13779] use provided parameters in the wait_for_pipeline_state hook
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #17137:
URL: https://github.com/apache/airflow/pull/17137#issuecomment-902930128
Awesome work, congrats on your first merged pull request!
--
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