Merge pull request #737 from cderici/charmhub-info-revisit

#737

#### Description

This PR revisits the `info` method in `charmhub.py`, allowing users to get information for a charm (like `juju info juju-qa-test`) in pylibjuju using either old or new clients. For old clients it calls `Info` from the `Charmhub` facade, and for the new clients it directly calls the charmhub api and processes the result. The channel-map representations are different between the `2.9` and `3.0` clients, this change introduces extra effort to get them as close as possible.

Fixes #734 


#### QA Steps

The existing charmhub tests are updated, and the ones that we used to skip (due to charmhub facade not existing anymore) are re-activated since we have the working code now (only the `info` ones). QA should be done for both `2.9` and `3.0` clients. So get yourself `juju 2.9` and `juju 3.0` and bootstrap two controllers and run the relevant tests:

```
tox -e integration -- tests/integration/test_charmhub.py::test_info
```
```
tox -e integration -- tests/integration/test_charmhub.py::test_info_with_channel
```
```
tox -e integration -- tests/integration/test_charmhub.py::test_info_not_found
```

You may also play with the `examples/charmhub_info.py` with different controllers if you wanna be extra pedantic.

Author: jujubot

juju/charmhub.py

@@ -10,31 +10,33 @@ class CharmHub:
     def __init__(self, model):
         self.model = model
 
-    def request_charmhub_with_retry(self, url, retries):
+    async def _charmhub_url(self):
+        model_conf = await self.model.get_config()
+        return model_conf['charmhub-url']
+
+    async def request_charmhub_with_retry(self, url, retries):
         for attempt in range(retries):
             _response = requests.get(url)
             if _response.status_code == 200:
                 return _response
-            jasyncio.sleep(5)
+            await jasyncio.sleep(5)
         raise JujuError("Got {} from {}".format(_response.status_code, url))
 
     async def get_charm_id(self, charm_name):
         conn, headers, path_prefix = self.model.connection().https_connection()
 
-        model_conf = await self.model.get_config()
-        charmhub_url = model_conf['charmhub-url']
+        charmhub_url = await self._charmhub_url()
         url = "{}/v2/charms/info/{}".format(charmhub_url.value, charm_name)
-        _response = self.request_charmhub_with_retry(url, 5)
+        _response = await self.request_charmhub_with_retry(url, 5)
         response = json.loads(_response.text)
         return response['id'], response['name']
 
     async def is_subordinate(self, charm_name):
         conn, headers, path_prefix = self.model.connection().https_connection()
 
-        model_conf = await self.model.get_config()
-        charmhub_url = model_conf['charmhub-url']
+        charmhub_url = await self._charmhub_url()
         url = "{}/v2/charms/info/{}?fields=default-release.revision.subordinate".format(charmhub_url.value, charm_name)
-        _response = self.request_charmhub_with_retry(url, 5)
+        _response = await self.request_charmhub_with_retry(url, 5)
         response = json.loads(_response.text)
         return 'subordinate' in response['default-release']['revision']
 
@@ -44,10 +46,9 @@ async def is_subordinate(self, charm_name):
     async def list_resources(self, charm_name):
         conn, headers, path_prefix = self.model.connection().https_connection()
 
-        model_conf = await self.model.get_config()
-        charmhub_url = model_conf['charmhub-url']
+        charmhub_url = await self._charmhub_url()
         url = "{}/v2/charms/info/{}?fields=default-release.resources".format(charmhub_url.value, charm_name)
-        _response = self.request_charmhub_with_retry(url, 5)
+        _response = await self.request_charmhub_with_retry(url, 5)
         response = json.loads(_response.text)
         return response['default-release']['resources']
 
@@ -63,11 +64,89 @@ async def info(self, name, channel=None):
         if not name:
             raise JujuError("name expected")
 
-        if channel is None:
-            channel = ""
-
-        facade = self._facade()
-        return await facade.Info(tag="application-{}".format(name), channel=channel)
+        if self.model.connection().is_using_old_client:
+            if channel is None:
+                channel = ""
+            facade = self._facade()
+            res = await facade.Info(tag="application-{}".format(name),
+                                    channel=channel)
+            err_code = res.errors.error_list.code
+            if err_code:
+                raise JujuError(f'charmhub.info - {err_code} :'
+                                f' {res.errors.error_list.message}')
+            result = res.result
+            result.channel_map = CharmHub._channel_map_to_dict(
+                result.channel_map,
+                name,
+                channel=channel)
+            result = result.serialize()
+        else:
+            charmhub_url = await self._charmhub_url()
+            url = "{}/v2/charms/info/{}?fields=channel-map".format(
+                charmhub_url.value, name)
+            try:
+                _response = await self.request_charmhub_with_retry(url, 5)
+            except JujuError as e:
+                if '404' in e.message:
+                    raise JujuError(f'{name} not found') from e
+            result = json.loads(_response.text)
+            result['channel-map'] = CharmHub._channel_list_to_map(result['channel-map'],
+                                                                  name,
+                                                                  channel=channel)
+        return result
+
+    @staticmethod
+    def _channel_list_to_map(channel_list_map, name, channel=""):
+        """Charmhub API returns the channel map as a list of channel objects
+        (with risk, track, revision, download etc). This turns that into a map
+        that's keyed with the channel=track/risk for easy
+        filtering/processing. This representation is also closer to the
+        result of the 2.9 facade call.
+
+        So basically,
+        [{'channel':{'risk':'stable', 'track':'latest'}, 'revision': 58}]
+          becomes:
+        {'latest/stable': {'channel':{'risk':'stable', 'track':'latest'},
+        'revision': 58}}
+
+        :param channel_list_map: [map[str][any]]
+        :return: map[str][map[str][any]]
+        """
+        channel_map = {}
+        for ch in channel_list_map:
+            ch_name = f"{ch['channel']['track']}/{ch['channel']['risk']}"
+            if channel and channel != ch_name:
+                # If channel is given, then filter out the rest
+                continue
+            channel_map[ch_name] = ch
+            if channel == ch_name:
+                # If we found the desired channel, no need to continue
+                break
+        # After loop is done, check for non-existent channel
+        if channel and channel not in channel_map:
+            raise JujuError(f'Charmhub.info : channel {channel} not found for'
+                            f' {name}')
+        return channel_map
+
+    @staticmethod
+    def _channel_map_to_dict(channel_map, name, channel=""):
+        """Converts the client.definitions.Channel objects into python maps
+        inside a channel map (for pylibjuju <3.0)
+
+        :param channel_map: map[str][Channel]
+        :return: map[str][map[str][any]]
+        """
+        channel_dict = {}
+        for ch_name, ch_obj in channel_map.items():
+            # No need to worry about filtering channel
+            # Charmhub facade will take care of that
+            _ch = ch_obj.serialize()
+            _ch['platforms'] = [p.serialize() for p in _ch['platforms']]
+            channel_dict[ch_name] = _ch
+        if channel and channel not in channel_dict:
+            raise JujuError(f'Charmhub.info : channel {channel} not found for'
+                            f' {name}')
+        return channel_dict
 
     async def find(self, query, category=None, channel=None,
                    charm_type=None, platforms=None, publisher=None,

tests/integration/test_charmhub.py

@@ -1,7 +1,7 @@
 import pytest
 
 from .. import base
-from juju.errors import JujuAPIError, JujuError
+from juju.errors import JujuError
 from juju import jasyncio
 
 
@@ -12,29 +12,47 @@ async def test_info(event_loop):
         _, name = await model.charmhub.get_charm_id("hello-juju")
         assert name == "hello-juju"
 
+        charm_name = 'juju-qa-test'
+        charm_info = await model.charmhub.info(charm_name)
+        assert charm_info['name'] == 'juju-qa-test'
+        assert charm_info['type'] == 'charm'
+        assert charm_info['id'] == 'Hw30RWzpUBnJLGtO71SX8VDWvd3WrjaJ'
+        assert '2.0/stable' in charm_info['channel-map']
+        cm_rev = charm_info['channel-map']['2.0/stable']['revision']
+        if type(cm_rev) == dict:
+            # New client (>= 3.0)
+            assert cm_rev['revision'] == 22
+        else:
+            # Old client (<= 2.9)
+            assert cm_rev == 22
+
 
 @base.bootstrapped
 @pytest.mark.asyncio
-@pytest.mark.skip('CharmHub facade no longer exists')
 async def test_info_with_channel(event_loop):
     async with base.CleanModel() as model:
-        result = await model.charmhub.info("hello-juju", "latest/stable")
+        charm_info = await model.charmhub.info("juju-qa-test", "2.0/stable")
+        assert charm_info['name'] == 'juju-qa-test'
+        assert '2.0/stable' in charm_info['channel-map']
+        assert 'latest/stable' not in charm_info['channel-map']
 
-        assert result.result.name == "hello-juju"
-        assert "latest/stable" in result.result.channel_map
+        try:
+            await model.charmhub.info("juju-qa-test", "non-existing-channel")
+        except JujuError as err:
+            assert err.message == 'Charmhub.info : channel ' \
+                                  'non-existing-channel not found for ' \
+                                  'juju-qa-test'
+        else:
+            assert False, "non-existing-channel didn't raise an error"
 
 
 @base.bootstrapped
 @pytest.mark.asyncio
-@pytest.mark.skip('CharmHub facade no longer exists')
 async def test_info_not_found(event_loop):
     async with base.CleanModel() as model:
-        try:
+        with pytest.raises(JujuError) as err:
             await model.charmhub.info("badnameforapp")
-        except JujuAPIError as e:
-            assert e.message == "badnameforapp not found"
-        else:
-            assert False
+            assert "badnameforapp not found" in str(err)
 
 
 @base.bootstrapped