-
Notifications
You must be signed in to change notification settings - Fork 14.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Snowpark operator and decorator #42457
Conversation
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)
|
b660a61
to
63ccc75
Compare
Could anyone please take a look? Thanks! |
I think there is a problem with dependencies that causes PROD build to fail |
Or simply rebase might fix it. |
Yeah I checked CI failures last week, it seems unrelated. I rebased once but got some new failures you saw here. @jscheffl Thanks for rebasing it! |
All merge gates are green now. Can you please take a look when you get a chance? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no real expertise with Snowflake/Snowpark, generally the cod elooks OK but can not judge functional correctness. Second eye review requested before merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as well. Though also - I do not know details of Snowflake integration.
Speaking of which.....
Maybe it is a good idea that Snowflake team implements System Tests and system test dashboard similar to the ones that Amazon, Google. Astronomer and Teradata did (see https://airflow.apache.org/ecosystem/#airflow-provider-system-test-dashboards )
We had a talk about it at Airflow Summit (recording will be published soon) where the authors of such dashboard explained what benefit it brings to them and that might be a great way to make sure that the quality of integration with Snowflake is maintained over time.
See https://airflowsummit.org/sessions/2024/hello-quality-building-cis-to-run-providers-packages-system-tests/ for description of the talk (recordings should be avaoilable at this link soon)
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
Yes! @mik-laj already told me that and I'm working on such a dashboard now |
👀 👀 👀 👀 👀 👀 👀 👀 |
closes: #24456
This is the first step of adding Snowpark-related operators and decorators. We will add more in the following PRs (e.g., running snowpark in virtual env). This PR also includes unit tests, system tests and documentation changes.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named
{pr_number}.significant.rst
or{issue_number}.significant.rst
, in newsfragments.