Refactor query status polling logic in EMRContainerHook

[victorphoenix3]

Query status polling logic currently polls until the query is not in intermmediate state. It should instead poll until query is not in the terminal state.
Resolves #19877

[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: dev@airflow.apache.org
  Slack: https://s.apache.org/airflow-slack

[ferruzzi]

Is "CANCEL_PENDING" really a terminal state?

Looks good to me other than this question.

[victorphoenix3]

"CANCEL_PENDING" is being treated as a failure state for the EMR container sensor, which is why I included it as a terminal state. Should it be removed?

[ferruzzi]

I don't know the EMR behavior well enough to say. If you feel it's intentional and the right thing to do, then leave it in. Just asking because the name strikes me as an intermediate state.

[victorphoenix3]

Going by the code here and here, I'll leave it in as a terminal state.

[victorphoenix3]

@ferruzzi A similar polling function exists for Athena queries as well [link]. Should I modify that too?

[ferruzzi]

@victorphoenix3 Yeah, it looks like the exact same issue. If you are up for it, then by all means! But I'd say drop it in a fresh PR so you don't tie this one up.

[victorphoenix3]

@ferruzzi Here's the separate PR for athena queries. Both PRs are complete, waiting for your approval. Thanks!

[ferruzzi]

I like it. You'll have to wait for a Committer to approve and merge, but I don't see any issues.

Maybe edit the top post to link to the Athena one as well. A reviewer might see that and take them both at once.

[potiuk]

Is it possible to add a unit test for that one (and rebase it ? ) I am happy to approve it but some unit testing could prevent regressions.

[victorphoenix3]

@potiuk I have added the unit tests, excuse the delay. Please review.

[potiuk]

The failing tests were unrelated (I am fixing them in a separate branch and updated constraints to get rid of those errors). I rebased the PR to latest main to re-test it.

[boring-cyborg]

Awesome work, congrats on your first merged pull request!