Skip to content

Conversation

@subbota19
Copy link
Contributor


Fix #46648.

Problem Description
In deferable mode, Airflow uses triggers to check the job status, so every time the deferable interval is reached, it checks the status and yields an event on this line.

However, when not using deferable logic, the execution follows the else statement, which does not include a while loop to continuously monitor execution. Instead, on this line, the task iterates through active Snowflake jobs, checks their status and then sleeps for poll_interval.

This results in the following behavior:

✅ If a job finishes in less than 5 seconds ( time.sleep using the default poll_interval), the logic works correctly.
❌ If a job takes longer than 5 seconds, there is no mechanism to keep monitoring it, and the task incorrectly finishes after the loop ends - even if the Snowflake job is still running.

Suggested Fix
I propose adding a running status check inside poll_on_queries and modifying the else statement with a loop to ensure Airflow waits until all queries finish, the logic is quick similar to what we use in deferable trigger while loop

With this change, the Airflow task would periodically call poll_on_queries and continue waiting until all jobs are complete.

Tests
Simulates a scenario where the Snowflake job is running for two checks (10 seconds apart, assuming default poll_interval): ffter the second check, the job is completed successfully and task is marked as successful.

Simulates a scenario where the Snowflake job failed and ensures AirflowException is raised.

@boring-cyborg boring-cyborg bot added the provider:snowflake Issues related to Snowflake provider label Feb 12, 2025
@boring-cyborg
Copy link

boring-cyborg bot commented Feb 12, 2025

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 Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:

  • Pay attention to the quality of your code (ruff, 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.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    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

@potiuk
Copy link
Member

potiuk commented Feb 17, 2025

You need to resolve conflicts now

@subbota19
Copy link
Contributor Author

Hi @potiuk!
I've resolved the conflict on my end, could you please double-check it? Thanks!

@subbota19
Copy link
Contributor Author

Hi @rawwar !
Could you please review this PR again and resolve the remaining conversations? Thanks!

@rawwar
Copy link
Contributor

rawwar commented Feb 24, 2025

Hi @rawwar ! Could you please review this PR again and resolve the remaining conversations? Thanks!

I think you should be able to mark the conversations as resolved. If not, a committer will be able to help. I don't have permission to mark the conversation as resolved.

@subbota19
Copy link
Contributor Author

Hi @potiuk!
I've resolved the conflict on my end and added suggestions, could you please double-check it? Thanks!

@rawwar
Copy link
Contributor

rawwar commented Mar 1, 2025

Hi @potiuk! I've resolved the conflict on my end and added suggestions, could you please double-check it? Thanks!

@subbota19 , take changes from main. test failures are already fixed.

… Before Snowflake Job Completes (apache#46648): implementation & tests
@subbota19
Copy link
Contributor Author

Hi @potiuk! I've resolved the conflict on my end and added suggestions, could you please double-check it? Thanks!

@subbota19 , take changes from main. test failures are already fixed.

Hi @rawwar, thx, I've rebased to the main branch

@subbota19
Copy link
Contributor Author

Hi @eladkal @potiuk !
I believe we can go ahead and merge this PR

@potiuk potiuk merged commit b275e79 into apache:main Mar 6, 2025
60 checks passed
@boring-cyborg
Copy link

boring-cyborg bot commented Mar 6, 2025

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

nailo2c pushed a commit to nailo2c/airflow that referenced this pull request Apr 4, 2025
… Before Snowflake Job Completes (apache#46648): implementation & tests (apache#46672)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

provider:snowflake Issues related to Snowflake provider

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SnowflakeSqlApiOperator(without deferable flag) marks Task as Success Before Snowflake Job Completes

4 participants