Skip to content

Commit

Permalink
Merge pull request #647 from cderici/fix-charm-origin-default-value
Browse files Browse the repository at this point in the history
#647

#### Description

This is a quick fix for a bug we seem to have introduced in #633 . Empty string is not a valid CharmOrigin value.

Update: turns out in #633, I introduced a bug where I didn't realize the newer versions of Juju using the `CharmsFacade` returns a `CharmOriginResult` from an `AddCharm` call, while the `ClientFacade` in the older versions returns just a dictionary. This is why when we try to `"get"` the `charm_origin` from the object we get a `None`. (this is why people like static typing)

This also fixes [LP #1961328](https://bugs.launchpad.net/juju/+bug/1961328). We're able to reproduce the error (thanks to @sed-i ), along with confirming that this PR fixes it (see QA).

#### QA Steps

* All the CI tests should pass (modulo the intermittent CI fails).
* The following should work:
```bash
# juju @ git+https://github.com/juju/python-libjuju.git@9ef8e6ad2d4e805250265bf0a1f34a57918a3836
> git clone https://github.com/canonical/alertmanager-k8s-operator.git
> cd alertmanager-k8s-operator
> git checkout 279f4bf
> tox -e integration -- -k test_upgrade_charm
```
* Also just to make sure we're not regressing here, this QA should include the QA steps for #633. (I did qa that with this fix and it seems to be working, though it'd help whoever's QAing this do that as well)

#### Notes & Discussion
  • Loading branch information
jujubot authored Mar 9, 2022
2 parents 9ef8e6a + fe2e4eb commit ea8f5a6
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion juju/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -1608,7 +1608,12 @@ async def deploy(
# actually support them yet anyway
if not res.is_local:
add_charm_res = await self._add_charm(identifier, res.origin)
charm_origin = add_charm_res.get('charm_origin', '')
if isinstance(add_charm_res, dict):
# This is for backwards compatibility for older
# versions where AddCharm returns a dictionary
charm_origin = add_charm_res.get('charm_origin', res.origin)
else:
charm_origin = add_charm_res.charm_origin

if Schema.CHARM_HUB.matches(url.schema):
resources = await self._add_charmhub_resources(res.app_name,
Expand Down

0 comments on commit ea8f5a6

Please sign in to comment.