Skip to content
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

Switch from Poetry to uv #549

Closed
wants to merge 1 commit into from
Closed

Switch from Poetry to uv #549

wants to merge 1 commit into from

Conversation

danyeaw
Copy link
Member

@danyeaw danyeaw commented Oct 5, 2024

Switch to uv to improve environment set up performance.

The only limitation I ran in to is I wasn't able to get the lock file integrity check to work with the exceptiongroup dependency only applicable for older Python versions.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Documentation content changes

What is the current behavior?

Poetry is used for dependency management and environment management

Issue Number: N/A

What is the new behavior?

uv is now used.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@danyeaw danyeaw force-pushed the poetry-to-uv branch 5 times, most recently from 610c8cf to 2ade5cb Compare October 5, 2024 23:18
@danyeaw
Copy link
Member Author

danyeaw commented Oct 10, 2024

@amolenaar I am not able to access the snyx test failure information.

@danyeaw danyeaw added the chore label Oct 10, 2024
Copy link
Member

@amolenaar amolenaar left a comment

Choose a reason for hiding this comment

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

This PR contains many good improvements: tightening security, version updates.

I do not see the need to switch to uv, however. Poetry serves us well, and makes lock files that work on all platforms.

generic/event.py Outdated
@@ -57,7 +57,7 @@ def handle(self, event: Event) -> None:
for handler in set(handler_set):
try:
handler(event)
except BaseException as e:
except Exception as e:
Copy link
Member

Choose a reason for hiding this comment

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

I used BaseException here on purpose, so errors are also caught.

@danyeaw
Copy link
Member Author

danyeaw commented Oct 10, 2024

I was trying this out to have that discussion. Do you see better cross platform support as an advantage of Poetry? I wasn't aware of any cross platform issues with uv.

Definitely the speed of going from zero Python to a fully set up development environment is an advantage for uv, it is game changing fast. On CI even with caching the run time was reduced from 1m 45s to 1m 8s, so about 35% less time running the CI pipeline.

A tradeoff is we are relying on Astral which is a startup to build tools in Rust. They are getting a lot of outside contributions, but it does probably make it more difficult for us to contribute PRs if we find an issue.

@danyeaw danyeaw closed this Oct 20, 2024
@danyeaw danyeaw deleted the poetry-to-uv branch November 25, 2024 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants