Skip to content

Conversation

@lmmx
Copy link
Contributor

@lmmx lmmx commented Apr 10, 2025

This PR refactors the Python rewrite package manager choice logic using an enum to make the decision tree explicitly clear, and thus also to allow a particular branch to be specified in config

Tasks

  • refactors the Python providers module taking care to preserve the exact pre-existing behaviour (so no existing projects will choose a different package manager to install from)
  • Tests are added/updated
    • Unit tests to correctly resolve config variable for NIXPACKS_PYTHON_PACKAGE_MANAGER as "poetry", "uv", etc.
  • Docs are updated

@lmmx lmmx changed the title refactor(providers): use enum for Python package manager choice refactor(providers): specify NIXPACKS_PYTHON_PACKAGE_MANAGER Apr 10, 2025
@lmmx lmmx marked this pull request as ready for review April 10, 2025 22:34
@madisvain
Copy link

Currently, when both requirements.txt and uv.lock exist in a project, the system automatically prioritizes pip (via requirements.txt) because of the fixed priority order in the code. I need to keep requirements.txt in my project for general pip compatibility, but specifically on Railway I'd like to try using uv instead. This PR would allow overriding the default package manager selection through the NIXPACKS_PYTHON_PACKAGE_MANAGER configuration variable, making it possible to choose uv even when requirements.txt is present.

@coffee-cup coffee-cup added the release/minor Author minor release label Apr 24, 2025
Copy link
Contributor

@coffee-cup coffee-cup left a comment

Choose a reason for hiding this comment

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

This looks great! ty

@lmmx
Copy link
Contributor Author

lmmx commented Apr 24, 2025

yw, will take a look at the clippy lint failure my bad sorry!

@lmmx
Copy link
Contributor Author

lmmx commented Apr 25, 2025

Reviewable once more 🫡

@coffee-cup
Copy link
Contributor

Just a few more linting failures. You should be able to fix these automatically (or see where they are) with cargo lint-fix locally. Make sure you are using the latest Rust version.

@lmmx lmmx force-pushed the feat-specify-pref-pkg-mgr branch from a1c6264 to 82936aa Compare April 25, 2025 20:59
@lmmx
Copy link
Contributor Author

lmmx commented Apr 25, 2025

I just see one, think I got it!

@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2025

This pull request is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the stale The pull request is stale label May 6, 2025
@lmmx lmmx force-pushed the feat-specify-pref-pkg-mgr branch from 82936aa to 4c00fc1 Compare May 6, 2025 16:07
@lmmx
Copy link
Contributor Author

lmmx commented May 6, 2025

This pull request is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days

Ready to review, not stale! @coffee-cup

@github-actions github-actions bot removed the stale The pull request is stale label May 7, 2025
@coffee-cup coffee-cup merged commit 34bd824 into railwayapp:main May 12, 2025
104 checks passed
@lmmx lmmx deleted the feat-specify-pref-pkg-mgr branch May 12, 2025 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release/minor Author minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants