[JUJU-3927] Add 3.2.0 facades #874
Merged
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
|
Running locally I got:
|
Yeah I'm trying to run these as well, for some reason my python environment just blew up trying to recover 🤦 |
cderici
approved these changes
Jun 6, 2023
There was a problem hiding this comment.
Yeah the regression confirmed for the tests/integration/test_controller.py::test_list_models_user_access. It passes against the 3.1 and fails in 3.2. Looking closely, this seems to be a regression in the ModelManagerFacade.ListModels() in Juju. Created a ticket for that.
I approve this PR because 1) the regression is not about libjuju itself, and 2) everything else seems to be working fine. I manually ran all the failing tests and they seem to be working individually, so we're good to go for 3.2 👍
|
As you already mentioned, I will ignore the regression on the Juju side and move this PR forward. Thanks for double checking! |
jujubot
added a commit
that referenced
this pull request
Jun 15, 2023
#882 #### Description This is something that we [stumbled upon](#874 (comment)) in one of the integration tests, which initially led me to believe there might be a regression in `ModelManager` facade. However, I realized that the libjuju only deals with the controller level access and doesn't know anything about model or offer access levels. There are two main contributions here: 1) Changes the way the controller object gets the `model_uuid`s (i.e. `cmd list-models` on juju). The old way was to call the `ControllerFacade.AllModels()` for admin users and `ModelManager.ListModels()` for anyone else. This changes it by calling `ModelManager.ListModelSummaries` for everyone just like [how `cmd` does it](https://github.com/juju/juju/blob/576c487266443c1b3d788bd1c85c4a45d0f91db0/cmd/juju/controller/listmodels.go#L126). 2) Adds new module `access.py` that has all the access levels for models, controllers and offers that can be used with `user.grant()` and `user.revoke()`. 3) Improves the existing `user.grant()` and `user.revoke()` which together with (2) should give a more fine grained control over permissions for pylibjuju users to test their stuff in setups involving multiple models, relations and users. #### QA Steps This revisits the existing integration tests and adds a new one too, so there are two main ways to QA this, manual, and automatic. The automatic way is to run the integration tests: ``` tox -e integration -- tests/integration/test_controller.py::test_grant_revoke_controller_access ``` ``` tox -e integration -- tests/integration/test_controller.py::test_grant_revoke_model_access ``` All CI tests need to pass. The manual way is to fire up a pylibjuju console and : ```python $ python -m asyncio asyncio REPL 3.10.6 (main, May 29 2023, 11:10:38) [GCC 11.3.0] on linux Use "await" directly instead of "asyncio.run()". Type "help", "copyright", "credits" or "license" for more information. >>> import asyncio >>> from juju import controller;c = controller.Controller(); await c.connect() >>> user = await c.add_user('foouser') >>> await c.list_models(username=user.username) [] >>> await user.grant('read', model_name='controller') True >>> await c.list_models(username=user.username) ['controller'] >>> await user.revoke('read', model_name='controller') True >>> await c.list_models(username=user.username) [] >>> ``` It is advisable to QA also the offer stuff as well.
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
This PR adds the latest facades from Juju 3.2.0, and prepares GitHub actions to use 3.2/stable
QA Steps
GitHub actions should be green. In any case, local testing should work: