-
Notifications
You must be signed in to change notification settings - Fork 53
fix: avoid return from finally block to fix Python 3.14 SyntaxWarning #361
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
Conversation
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.
1 file reviewed, 1 comment
9d2e454 to
1d8b0f1
Compare
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.
Pull request overview
This PR fixes a SyntaxWarning that occurs when running the posthog library on Python 3.14 by moving a return statement from inside a finally block to outside of it.
Key Changes:
- Moved the
return successstatement from inside thefinallyblock to after it in theupload()method
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pauldambra
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.
1d8b0f1 to
8ac3073
Compare
|
I've rebased the PR and made it log and swallow any errors from the |
pauldambra
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.
the log is a good touch for a previously totally silent error 👍
|
@Piccirello i can't merge here because i'm guessing that's because it's external? you-are-our-only-hope.meme |
|
CodeQL requires you to setup CodeQL Advanced in a repo in order to run it against external contributor PRs (super annoying). We've started setting this up in repos as it pops up, like in posthog, posthog-android, posthog-ios. #405 should fix this. |
|
@jodal, I've merged @Piccirello's #405, but we now need you to merge your branch with the current master to get CodeQL to run. Do you mind doing that? |
|
one more ping, i can't figure out how to update this to master myself, sorry! |
This fixes a SyntaxWarning on Python 3.14. ``` ❯ uvx --no-cache --python 3.14.0 --with posthog==6.7.11 python -c "import posthog" Installed 11 packages in 5ms .../lib/python3.14/site-packages/posthog/consumer.py:92: SyntaxWarning: 'return' in a 'finally' block return success ````
ae32dfc to
9ec7ca7
Compare
|
Rebased again. |
|
I've enabled the repo setting "Always suggest updating pull request branches", which will now show the “update branch” option. |
This fixes a SyntaxWarning on Python 3.14.