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

feat(python): Multiple runtime versions #4579

Merged
merged 86 commits into from
Jan 24, 2025
Merged

feat(python): Multiple runtime versions #4579

merged 86 commits into from
Jan 24, 2025

Conversation

pyranota
Copy link
Collaborator

@pyranota pyranota commented Oct 24, 2024

Allows to choose specific python version used for python scripts.

Version for script can be annotated with py310, py311, py312 or py313 annotations.
Or global version by setting INSTANCE_PYTHON_VERSION to one of the mentioned versions above or to "Latest Stable".

For newly deployed scripts annotated version will be assigned to lockfile and all future executions will respect that version.
If none specified, 3.11 will be used (even if instance version changed to something else)

For test runs or deploys, if there are imported scripts, wmill will search through all of them and use found annotated version as final one, otherwise use instance version. It is possible to annotate only ONE runtime version in single tree.

For EE customers, S3 cache tarballs will be separated by py-version.


Important

Add support for multiple Python runtime versions, allowing specification via annotations or instance settings, with backend and frontend updates.

  • Behavior:
    • Support for specifying Python version via annotations (py310, py311, py312, py313) or INSTANCE_PYTHON_VERSION.
    • Default to Python 3.11 if no version is specified.
    • For EE customers, S3 cache tarballs are separated by Python version.
  • Backend Changes:
    • Updated parse_python_imports in windmill-parser-py-imports/src/lib.rs to handle version annotations.
    • Added PyVersion enum in python_executor.rs to manage Python versions.
    • Modified handle_python_deps and python_dep to respect annotated and instance Python versions.
  • Frontend Changes:
    • Added Instance Python Version setting in instanceSettings.ts and InstanceSetting.svelte.
    • Utilized ToggleButtonGroup for selecting Python versions.
  • Misc:
    • Updated Dockerfile to preinstall specified Python versions.
    • Removed uv from shell.nix dependencies.

This description was created by Ellipsis for b7c6d14. It will automatically update as commits are pushed.

Copy link

cloudflare-workers-and-pages bot commented Oct 24, 2024

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: 045d2be
Status: ✅  Deploy successful!
Preview URL: https://4be33e17.windmill.pages.dev
Branch Preview URL: https://multipython.windmill.pages.dev

View logs

- Added instance python version
- Rework logic
error[E0599]: no method named `iter` found for tuple `(PyVersion, std::vec::Vec<std::string::String>)` in the current scope
config file missed /proc mount causing install phase to fail
@pyranota
Copy link
Collaborator Author

/buildimage_nsjail

@pyranota pyranota marked this pull request as ready for review December 19, 2024 18:59
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 060fe92 in 1 minute and 48 seconds

More details
  • Looked at 1818 lines of code in 20 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. shell.nix:32
  • Draft comment:
    The uv package is commented out, but the PR description mentions its usage for Python runtime management. Ensure that uv is included if it's required for the new feature.
  • Reason this comment was not posted:
    Comment did not seem useful.

Workflow ID: wflow_ZUSeqHwqBHnurdkg


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on ad84b24 in 58 seconds

More details
  • Looked at 46 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. backend/windmill-worker/src/python_executor.rs:298
  • Draft comment:
    Using unwrap_or_else with a panic message can cause the application to crash. Consider handling errors more gracefully to avoid unexpected panics. This is applicable in other places as well, such as lines 46, 49, 52, and 53.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.
2. backend/windmill-worker/src/python_executor.rs:333
  • Draft comment:
    Using unwrap_or_else with a panic message can cause the application to crash. Consider handling errors more gracefully to avoid unexpected panics. This is applicable in other places as well, such as lines 46, 49, 52, and 53.
  • Reason this comment was not posted:
    Marked as duplicate.
3. backend/windmill-worker/src/python_executor.rs:1527
  • Draft comment:
    Using unwrap_or_else with a panic message can cause the application to crash. Consider handling errors more gracefully to avoid unexpected panics. This is applicable in other places as well, such as lines 46, 49, 52, and 53.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_YFvJBi13qG7hxjqJ


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@pyranota pyranota marked this pull request as draft January 17, 2025 17:45
@pyranota
Copy link
Collaborator Author

  • Check if test runs with provided requirements.txt can be confused with deployed

@pyranota pyranota marked this pull request as ready for review January 23, 2025 20:15
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to b7c6d14 in 1 minute and 38 seconds

More details
  • Looked at 1788 lines of code in 19 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. shell.nix:32
  • Draft comment:
    Consider uncommenting the uv package if it's required for the Python runtime management feature. If it's intentionally commented out, ensure that the feature works without it.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces a new feature to handle multiple Python runtime versions. The changes in the codebase are extensive, and it's crucial to ensure that the logic for selecting and managing Python versions is correctly implemented. The shell.nix file has a commented-out line for uv, which might be intentional, but it's worth noting in case it's an oversight.
2. shell.nix:33
  • Draft comment:
    Consider uncommenting the uv package if it's required for the Python runtime management feature. If it's intentionally commented out, ensure that the feature works without it.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces a new feature to handle multiple Python runtime versions. The changes in the codebase are extensive, and it's crucial to ensure that the logic for selecting and managing Python versions is correctly implemented. The shell.nix file has a commented-out line for uv, which might be intentional, but it's worth noting in case it's an oversight.

Workflow ID: wflow_31q9iMEfgpxVC6QM


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@rubenfiszel rubenfiszel merged commit e47dd69 into main Jan 24, 2025
19 of 20 checks passed
@rubenfiszel rubenfiszel deleted the multipython branch January 24, 2025 10:42
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants