Change dependency for macaroonbakery to support protobuf >= 4.21.1 #956
Conversation
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
Fixes juju#914 by using a fork of macaroonbakery that's introduced in go-macaroon-bakery/py-macaroon-bakery#90
SimonRichardson
requested changes
Oct 6, 2023
| @@ -22,7 +22,8 @@ | |||
| exclude=["*.tests", "*.tests.*", "tests.*", "tests"]), | |||
| package_data={'juju': ['py.typed']}, | |||
| install_requires=[ | |||
| 'macaroonbakery>=1.1,<2.0', | |||
| # see https://github.com/juju/python-libjuju/issues/914 for macaroon dependency | |||
| 'macaroonbakery@git+https://github.com/cderici/py-macaroon-bakery@relax-protobuf-version#egg=macaroonbakery', | |||
There was a problem hiding this comment.
We need to fork that into the juju namespace.
There was a problem hiding this comment.
@SimonRichardson I'd steer away from that because in my mind this is mostly a temporary solution, mostly because 1) I don't want us to maintain that, 2) macaroonbakery library hasn't been touched since 2020, so we gotta find another more up-to-date library for that, wdyt?
|
This PR is marked as incomplete because it has been open 30 days with no activity. Please remove incomplete label or comment or this will be closed in 5 days. |
cderici
added
work-in-progress
and removed
state/incomplete
|
No need for this anymore since upstream changed the protobuf requirements go-macaroon-bakery/py-macaroon-bakery#90 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Here's what this is trying to fix:
macaroonbakerylibrary hasn't been getting much love since 2020, and currently depends onprotobuf>=3.4.0,<4.0mysql-connector-pythonlibrary latest version depends onprotobuf>=4.21.1.pylibjujudepends onmacaroonbakery.Therefore anyone wants to use
pylibjujualong withmysql-connector-pythonin their project can't satisfyprotobufversion constraints.So I made a fork of macaroon bakery that relaxes the protobuf version, and pylibjuju points to that. I also created a pull request to the upstream repo. go-macaroon-bakery/py-macaroon-bakery#90 . If that lands, then this dependency can go back to using
pypi. But until then, pylibjuju needs to point to that fork in order to be compatible withmysql-connector-python.Fixes #914
QA Steps
Dependency can be tested by the QA from #914, such as:
Steps to reproduce:
poetry new foocd foopoetry add mysql-connector-python^8.1.0poetry add juju>=2Pylibjuju should work as normal. macaroonbakery is used in
client/connectionso it's pretty central. If there's a problem, we'd definitely see it in the tests.Notes & Discussion
JUJU-4620