-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Install text-generation-server
from poetry.lock
export
#2786
Open
alvarobartt
wants to merge
5
commits into
main
Choose a base branch
from
install-from-poetry-lock
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
ebed60b
Add `poetry.lock` export into `requirements_poetry.txt`
alvarobartt d9347f2
Fix stage name in `Dockerfile`
alvarobartt a221dbb
Fix `COPY` destination for `requirements_poetry.txt`
alvarobartt bbde434
Remove commas from `poetry export --extras ...`
alvarobartt 63b8c59
Add `poetry-plugin-export` and fix indentation
alvarobartt File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
What's the issue again ? Why are we not using poetry directly ?
Also maybe we should take the opportunity to switch to
uv
? https://docs.astral.sh/uv/pip/compile/#locking-requirements I have no idea, but it seems like the poetry lock isn't locking tightly enough, maybe ?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.
Sorry for the delay on this PR, and happy new year BTW! So the issue is that since we're not using
poetry
as the pip installer the locked dependencies are not used leading to potential issues when partners as e.g. Google build the containers ahead in time, as those dependencies are installed from thepyproject.toml
rather than the lock.I know this is not an issue impacting directly TGI per se, but for robustness and reproducibility we should try to lock those and install those from a lock. The installation from the lock can either be via
poetry
(the current approach), or switching touv
instead and installing from theuv.lock
.Maybe we should start an internal conversation and decide what's best, happy to help whatever the solution / proposal is!