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

chore: Upgrade jedi to 0.19.1 #5693

Merged
merged 4 commits into from
Jun 28, 2024
Merged

Conversation

mattrunyon
Copy link
Contributor

Adds support for Python 3.11 and 3.12.

Also, the 0.19.0 changelog notes a massive performance improvement for Interpreter mode which we use by default. It felt a bit snappier to me while using it locally.

Also adds a new flag to avoid unsafe execution if we want to enable that. Not sure what the default is based on the changelog.

https://jedi.readthedocs.io/en/latest/docs/changelog.html

jmao-denver
jmao-denver previously approved these changes Jun 27, 2024
Copy link
Contributor

@jmao-denver jmao-denver left a comment

Choose a reason for hiding this comment

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

LGTM

@jmao-denver
Copy link
Contributor

Two questions:

  1. I see that we are a few months late (jedi 0.19 was released Oct 2023). This in itself is not a big deal, but I feel it might be better if we have a formal process to watch the releases of our dependencies and have a better control on when/why/why not update the pinned versions.
  2. In the absence of 1, should we not pin the version (or have an open-end up limit) for the core package installation requirement?

@@ -1,3 +1,3 @@
io.deephaven.project.ProjectType=DOCKER_REGISTRY
deephaven.registry.imageName=ghcr.io/deephaven/server-slim-base:edge
deephaven.registry.imageId=ghcr.io/deephaven/server-slim-base@sha256:ca9b55d2e075dc12e220b8cb13fdd9cd4cb7fc8f68dfdaba99fc240f5a916266
deephaven.registry.imageId=ghcr.io/deephaven/server-slim-base@sha256:8011b73d42aebf3d240c25e2b0a12c21dbc0049fa8444924fe9f9dbeb1ca9ddfs
Copy link
Member

Choose a reason for hiding this comment

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

This is not a valid digest. I think you added an extra "s" onto the end. Clearly, our CI build doesn't even use this image, otherwise it would have failed... :/

Copy link
Contributor

@jmao-denver jmao-denver left a comment

Choose a reason for hiding this comment

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

LGTM

@mattrunyon mattrunyon merged commit 5a81698 into deephaven:main Jun 28, 2024
16 checks passed
@mattrunyon mattrunyon deleted the jedi-upgrade branch June 28, 2024 20:56
@github-actions github-actions bot locked and limited conversation to collaborators Jun 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants