-
Notifications
You must be signed in to change notification settings - Fork 527
Fix Python 3.13+ TypeError by decoding bytes URLs in storage client #2682
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
base: main
Are you sure you want to change the base?
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
Related issue #2676 |
sfc-gh-fpawlowski
left a comment
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.
Hi, thanks for the PR!
Left one comment, but overall seems good.
| logger.debug(f"retry #{self.retry_count[retry_id]}") | ||
| cur_timestamp = self.credentials.timestamp | ||
| url, rest_kwargs = get_request_args() | ||
| url = url.decode('utf-8') if isinstance(url, bytes) else url |
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.
Could we move this check into the SessionManager.use_session? It should consistently accept both bytes and str as an input.
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.
thanks, done!
|
I have read the CLA Document and I hereby sign the CLA |
Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
No relevant GH issue.
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
Fixes a TypeError that occurs on Python 3.13+ when performing PUT file operations. The storage client generates URLs as bytes, but the session manager (and vendored requests library) expects strings. Python 3.13's stricter type enforcement for string concatenation exposes this mismatch.
The fix decodes bytes URLs to UTF-8 strings before passing them to
use_session(), ensuring type consistency across the request pipeline while maintaining backward compatibility with string URLs.Note: This issue is specific to the LocalStack Snowflake emulator environment and is not reproducible with real Snowflake services. However, the fix improves type safety and ensures proper adherence to the
use_session()type contract (which expectsstr, notbytes).