[Airflow 13779] use provided parameters in the wait_for_pipeline_state hook

[lwyszomi]

Related: #13779

Give a possibility to run wait_for_pipeline_state with default or provided parameters, remove unnecessary call inside start_pipeline hook.

Why I removed wait_for_pipeline_state from start_pipeline hook. By this call, I think we have a bug in this operator, for example when we have pipeline which starting more than 300 seconds, so it have a starting status, we get the error because this pipepline is not in correct state after 300 seconds. Even when we pass our parameters sucess_states and pipeline_timeout we get this error in this case, so I think when I pass both parameters the logic should use them not default. Why we have 300 second and these SUCCESS_STATES + [PipelineStates.RUNNING], because we had these values in the wait_for_pipeline_state call which I removed from hook, I think we should replace this 300 second and use default value from __init__ method (this is a open question I think)

---

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

[boring-cyborg]

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 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 Consider adding an example DAG that shows how users should use it.
• Consider using Breeze environment 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 for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
• Be sure to read the Airflow Coding style.
  Apache Airflow is a community-driven project and together we are making it better 🚀.
  In case of doubts contact the developers at:
  Mailing List: [email protected]
  Slack: https://s.apache.org/airflow-slack

[lwyszomi]

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.

[potiuk]

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.

[lwyszomi]

@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.

[potiuk]

@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 #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.

[lwyszomi]

@potiuk I rebased PR, I hope this help :) sorry that I ping you when you are on the vacation.

[github-actions]

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.

[lwyszomi]

@potiuk @uranusjr Can we trigger build maybe it will pass 😄

[uranusjr]

It’s stuck somehow.

[lwyszomi]

@uranusjr Thank you

[uranusjr]

Some checks are failing

[lwyszomi]

@uranusjr both checks failed because the timeouts.

[lwyszomi]

@uranusjr finally is green 😄

looks loke all addressed.

[boring-cyborg]

Awesome work, congrats on your first merged pull request!