-
Notifications
You must be signed in to change notification settings - Fork 51
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
prevent pip backtracking #356
Conversation
1090f4c
to
a251d59
Compare
Signed-off-by: Daniele Trifirò <[email protected]>
Signed-off-by: Daniele Trifirò <[email protected]>
@@ -17,6 +17,9 @@ dependencies = [ | |||
"caikit[runtime-grpc,runtime-http]>=0.26.17,<0.27.0", | |||
"caikit-tgis-backend>=0.1.27,<0.2.0", | |||
# TODO: loosen dependencies | |||
"grpcio>=1.62.2", # explicitly pin grpc dependencies to a recent version to avoid pip backtracking | |||
"grpcio-reflection>=1.62.2", | |||
"grpcio-health-checking>=1.62.2", |
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.
These are technically caikit
/caikit[runtime-grpc]
dependencies, and we tended to keep a fairly loose versioning system on caikit to allow any applications/users potentially specify tighter bounds.
I'm fine with this (forcing users on much more recent versions) in theory here since this library is likely not getting very wide use, but I am curious - how is this particular Dockerfile
/ image build currently being used? I presume https://github.com/opendatahub-io/caikit-nlp is the one building https://quay.io/repository/opendatahub/caikit-nlp?tab=tags ?
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.
it would be ideal if they are in pinned in caikit/ caikit[runtime-grpc]
, that way other dependent library also get it. Since grpc is internal to caikit-runtime
it kind of becomes confusing to add them here. But these can be updated here as an "override" for now though.
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.
We could pin those dependencies in caikit
instead if you'd prefer that, I preferred pinning them here because this is where the issue was encountered first, although I'm not sure if the backtracking is caused by caikit[runtime-grpc]
or itself some other dependency added by caikit-nlp. I can investigate if you'd prefer.
Regarding the Dockerfile
, that's correct, the image is the one published at quay.io/opendatahub/caikit-nlp
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.
yeah, the gRPC dependency is primarily coming from runtime
, but we can unblock the build here quickly by setting these overrides here. It may be possible that caikit
doesn't see this resolution slowdown, because it doesn't have conflicting deps, so it may as well need to be here as overrides!
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.
Looks good to me!
image builds are currently taking 5+h and failing because of pip backtracking.