Skip to content

Replaced deprecated logic in SPCS worker #18105

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

Merged
merged 4 commits into from
Jun 9, 2025

Conversation

bjorhn
Copy link
Contributor

@bjorhn bjorhn commented May 19, 2025

This PR updates the Snowpark Container Services worker to handle recent changes in the API of the Snowflake Python SDK:

  • Removed usage of deprecated method get_service_status()
  • Updated status checks to account for changes in status types returned by the API
  • Bumps Snowflake dependency to >= 1.5.0 (from >= 1.2.0)

There are no new unit tests since there is no new functionality.

This code is running in production at my workplace with no issues.

Closes #18104

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • If this pull request adds new functionality, it includes unit tests that cover the changes
  • If this pull request removes docs files, it includes redirect settings in mint.json.
  • If this pull request adds functions or classes, it includes helpful docstrings.

@github-actions github-actions bot added the enhancement An improvement of an existing feature label May 19, 2025
Copy link
Collaborator

@zzstoatzz zzstoatzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @bjorhn it looks like we need to update a test here, otherwise this LGTM

@zzstoatzz
Copy link
Collaborator

hi @bjorhn it looks like we need to update a test here, otherwise this LGTM

here's the failure im looking at

@bjorhn
Copy link
Contributor Author

bjorhn commented May 20, 2025

@zzstoatzz I'm not sure how any of my changes relate to that test, am I missing something? Is it possible that my snowflake dependency bump is forcing a higher version of snowflake-connector, which, in turn, causes the test to fail?

@zzstoatzz
Copy link
Collaborator

zzstoatzz commented May 20, 2025

@zzstoatzz I'm not sure how any of my changes relate to that test, am I missing something? Is it possible that my snowflake dependency bump is forcing a higher version of snowflake-connector, which, in turn, causes the test to fail?

@bjorhn yea i would guess the new versions just handle that PEM loading error a bit different so i suspect we'd just need to update the expected error message we're matching on now

if you get stuck on that I can take a look later

@bjorhn
Copy link
Contributor Author

bjorhn commented May 22, 2025

@zzstoatzz I had a quick look at this but at the moment I don't know when I can sit down and get this fixed. PEM loading isn't really my area of expertise. If you are able to take a look at this, please do, otherwise I will probably return to it at a later date.

@zzstoatzz
Copy link
Collaborator

ok @bjorhn I will take a look at this later this week

@zzstoatzz zzstoatzz force-pushed the snowflake-spcs-replaced-deprecated branch from 53b14d0 to fca0367 Compare June 6, 2025 19:45
@zzstoatzz zzstoatzz mentioned this pull request Jun 6, 2025
@zzstoatzz zzstoatzz force-pushed the snowflake-spcs-replaced-deprecated branch from fca0367 to 6c87175 Compare June 9, 2025 15:57
@zzstoatzz zzstoatzz force-pushed the snowflake-spcs-replaced-deprecated branch from 6c87175 to 36dc780 Compare June 9, 2025 15:58
@zzstoatzz
Copy link
Collaborator

the diff is massive here because of: astral-sh/uv#13176

its all just changing upload_time to upload-time - the latter is new and correct

Copy link
Collaborator

@zzstoatzz zzstoatzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you @bjorhn - apologies for the delay here

@zzstoatzz zzstoatzz merged commit b04a4c6 into PrefectHQ:main Jun 9, 2025
13 checks passed
@bjorhn bjorhn deleted the snowflake-spcs-replaced-deprecated branch June 18, 2025 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Snowpark Container Services worker to handle changes and deprecations in API
2 participants