Skip to content

[pip] Move protobuf pin from query to gear - #14393

Merged
hail-ci-robot merged 1 commit into
hail-is:mainfrom
daniel-goldstein:remove-protobuf-pin-v2
Mar 5, 2024
Merged

[pip] Move protobuf pin from query to gear#14393
hail-ci-robot merged 1 commit into
hail-is:mainfrom
daniel-goldstein:remove-protobuf-pin-v2

Conversation

@daniel-goldstein

@daniel-goldstein daniel-goldstein commented Mar 4, 2024

Copy link
Copy Markdown
Contributor

protobuf was pinned not because it is a direct dependency of hail but because the version of google-cloud-profiler that we use is incompatible with greater versions of protobuf despite not putting an upper bound on its pin. However, the current solution of pinning it in hailtop/requirements.txt and python/requirements.txt is problematic in three ways:

  1. Users of the hail pip package cannot use a newer version of protobuf if they want to
  2. The protobuf version is only a problem for google-cloud-profiler, which we only use in the batch/ci/auth services, not the hail package
  3. protobuf no longer a transitive dependency of anything in hailtop/hail, so why include it (and why include it twice?)

This is all fixable by moving the protobuf pin "up" to gear/requirements.txt, where we also install google-cloud-profiler. Whether or not we should be upgrading google-cloud-profiler to remove the protobuf pin is a valid but unfortunately thornier issue since google-cloud-profiler statically linking libc breaks libsass. This allows us to punt on that upgrade while freeing users of the protobuf restriction.

@hail-ci-robot
hail-ci-robot merged commit a26360c into hail-is:main Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants