add snap-http as a submodule

canonical · Jan 19, 2024 · c6fd45a · c6fd45a
1 parent d2847f6
commit c6fd45a
Show file tree

Hide file tree

Showing 15 changed files with 76 additions and 416 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -7,7 +7,9 @@ jobs:
       matrix:
         os: ["ubuntu-22.04", "ubuntu-20.04"]
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
+        with:
+          submodules: true
       - run: |
           make depends
           # -common seems a catch-22, but this is just a shortcut to
@@ -17,6 +19,8 @@ jobs:
   lint:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
+        with:
+          submodules: true
       - run: make depends
       - run: make lint
diff --git a/.github/workflows/codecov.yml b/.github/workflows/codecov.yml
@@ -7,6 +7,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
+        with:
+          submodules: true
       - run: |
           make depends
           # -common seems a catch-22, but this is just a shortcut to

diff --git a/.gitmodules b/.gitmodules
@@ -0,0 +1,3 @@
+[submodule "snap-http"]
+	path = snap-http
+	url = https://github.com/Perfect5th/snap-http
diff --git a/landscape/client/manager/snapmanager.py b/landscape/client/manager/snapmanager.py
@@ -1,24 +1,16 @@
+import json
 import logging
 from collections import deque
 
 from twisted.internet import task
 
+from landscape.client import snap_http
 from landscape.client.manager.plugin import FAILED
 from landscape.client.manager.plugin import ManagerPlugin
 from landscape.client.manager.plugin import SUCCEEDED
-
-try:
-    import snap_http
-    from snap_http import INCOMPLETE_STATUSES
-    from snap_http import SnapdHttpException
-    from snap_http import SUCCESS_STATUSES
-except ImportError:
-    from landscape.client.snap.http import INCOMPLETE_STATUSES
-    from landscape.client.snap.http import SnapdHttpException
-    from landscape.client.snap.http import SnapHttp
-    from landscape.client.snap.http import SUCCESS_STATUSES
-
-    snap_http = SnapHttp()
+from landscape.client.snap_http import INCOMPLETE_STATUSES
+from landscape.client.snap_http import SnapdHttpException
+from landscape.client.snap_http import SUCCESS_STATUSES
 
 
 class SnapManager(ManagerPlugin):
@@ -43,7 +35,7 @@ def __init__(self):
             "hold-snaps-batch": snap_http.hold_all,
             "unhold-snaps": snap_http.unhold,
             "unhold-snaps-batch": snap_http.unhold_all,
-            "set-config": snap_http.set_conf,
+            "set-snap-config": snap_http.set_conf,
         }
 
     def register(self, registry):
@@ -55,7 +47,7 @@ def register(self, registry):
         registry.register_message("refresh-snaps", self._handle_snap_task)
         registry.register_message("hold-snaps", self._handle_snap_task)
         registry.register_message("unhold-snaps", self._handle_snap_task)
-        registry.register_message("set-config", self._handle_snap_task)
+        registry.register_message("set-snap-config", self._handle_snap_task)
 
     def _handle_snap_task(self, message):
         """
@@ -96,7 +88,7 @@ def _handle_batch_snap_task(self, message):
             )
             queue.append((response["change"], "BATCH"))
         except SnapdHttpException as e:
-            result = e.json["result"]
+            result = json.loads(e.args[0])["result"]
             logging.error(
                 f"Error in {message_type}: {message}",
             )
@@ -127,7 +119,7 @@ def _handle_multiple_snap_tasks(self, message):
 
         # Naively doing this synchronously because each is an HTTP call to the
         # snap REST API that returns basically immediately. We poll for their
-        # completion statuses once they've all been kicked off.
+        # completion statuses once they've all been kicked off
         for snap in snaps:
             name = snap["name"]
             snap_args = snap.get("args", {})
@@ -140,7 +132,7 @@ def _handle_multiple_snap_tasks(self, message):
                 )
                 queue.append((response["change"], name))
             except SnapdHttpException as e:
-                result = e.json["result"]
+                result = json.loads(e.args[0])["result"]
                 logging.error(
                     f"Error in {message_type} for '{name}': {message}",
                 )
@@ -263,7 +255,7 @@ def _respond(self, snap_results, opid, errors):
 
     def _send_installed_snap_update(self):
         try:
-            installed_snaps = snap_http.list()
+            installed_snaps = snap_http.list().result
         except SnapdHttpException as e:
             logging.error(
                 f"Unable to list installed snaps after snap change: {e}",

diff --git a/landscape/client/manager/tests/test_snapmanager.py b/landscape/client/manager/tests/test_snapmanager.py
@@ -3,14 +3,11 @@
 from landscape.client.manager.manager import FAILED
 from landscape.client.manager.manager import SUCCEEDED
 from landscape.client.manager.snapmanager import SnapManager
+from landscape.client.snap_http import SnapdHttpException
+from landscape.client.snap_http import SnapdResponse
 from landscape.client.tests.helpers import LandscapeTest
 from landscape.client.tests.helpers import ManagerHelper
 
-try:
-    from snap_http import SnapdHttpException
-except ImportError:
-    from landscape.client.snap.http import SnapdHttpException
-
 
 class SnapManagerTest(LandscapeTest):
     helpers = [ManagerHelper]
@@ -56,7 +53,12 @@ def install_snap(name, revision=None, channel=None, classic=False):
                 {"id": "2", "status": "Done"},
             ],
         }
-        self.snap_http.list.return_value = {"installed": []}
+        self.snap_http.list.return_value = SnapdResponse(
+            "sync",
+            200,
+            "OK",
+            {"installed": []},
+        )
 
         result = self.manager.dispatch_message(
             {
@@ -94,19 +96,24 @@ def test_install_snaps_batch(self):
         self.snap_http.check_changes.return_value = {
             "result": [{"id": "1", "status": "Done"}],
         }
-        self.snap_http.list.return_value = {
-            "installed": [
-                {
-                    "name": "hello",
-                    "id": "test",
-                    "confinement": "strict",
-                    "tracking-channel": "latest/stable",
-                    "revision": "100",
-                    "publisher": {"validation": "yep", "username": "me"},
-                    "version": "1.2.3",
-                },
-            ],
-        }
+        self.snap_http.list.return_value = SnapdResponse(
+            "sync",
+            200,
+            "OK",
+            {
+                "installed": [
+                    {
+                        "name": "hello",
+                        "id": "test",
+                        "confinement": "strict",
+                        "tracking-channel": "latest/stable",
+                        "revision": "100",
+                        "publisher": {"validation": "yep", "username": "me"},
+                        "version": "1.2.3",
+                    },
+                ],
+            },
+        )
 
         result = self.manager.dispatch_message(
             {
@@ -139,7 +146,12 @@ def test_install_snap_immediate_error(self):
         self.snap_http.install_all.side_effect = SnapdHttpException(
             b'{"result": "whoops"}',
         )
-        self.snap_http.list.return_value = {"installed": []}
+        self.snap_http.list.return_value = SnapdResponse(
+            "sync",
+            200,
+            "OK",
+            {"installed": []},
+        )
 
         result = self.manager.dispatch_message(
             {
@@ -170,7 +182,9 @@ def got_result(r):
     def test_install_snap_no_status(self):
         self.snap_http.install_all.return_value = {"change": "1"}
         self.snap_http.check_changes.return_value = {"result": []}
-        self.snap_http.list.return_value = {"installed": []}
+        self.snap_http.list.return_value = SnapdResponse(
+            "sync", 200, "OK", {"installed": []}
+        )
 
         result = self.manager.dispatch_message(
             {
@@ -199,7 +213,9 @@ def got_result(r):
     def test_install_snap_check_error(self):
         self.snap_http.install_all.return_value = {"change": "1"}
         self.snap_http.check_changes.side_effect = SnapdHttpException("whoops")
-        self.snap_http.list.return_value = {"installed": []}
+        self.snap_http.list.return_value = SnapdResponse(
+            "sync", 200, "OK", {"installed": []}
+        )
 
         result = self.manager.dispatch_message(
             {
@@ -232,7 +248,9 @@ def test_remove_snap(self):
         self.snap_http.check_changes.return_value = {
             "result": [{"id": "1", "status": "Done"}],
         }
-        self.snap_http.list.return_value = {"installed": []}
+        self.snap_http.list.return_value = SnapdResponse(
+            "sync", 200, "OK", {"installed": []}
+        )
 
         result = self.manager.dispatch_message(
             {
@@ -263,11 +281,13 @@ def test_set_config(self):
         self.snap_http.check_changes.return_value = {
             "result": [{"id": "1", "status": "Done"}],
         }
-        self.snap_http.list.return_value = {"installed": []}
+        self.snap_http.list.return_value = SnapdResponse(
+            "sync", 200, "OK", {"installed": []}
+        )
 
         result = self.manager.dispatch_message(
             {
-                "type": "set-config",
+                "type": "set-snap-config",
                 "operation-id": 123,
                 "snaps": [
                     {

diff --git a/landscape/client/monitor/snapmonitor.py b/landscape/client/monitor/snapmonitor.py
@@ -1,17 +1,10 @@
 import logging
 
+from landscape.client import snap_http
 from landscape.client.monitor.plugin import DataWatcher
+from landscape.client.snap_http import SnapdHttpException
 from landscape.message_schemas.server_bound import SNAPS
 
-try:
-    import snap_http
-    from snap_http import SnapdHttpException
-except ImportError:
-    from landscape.client.snap.http import SnapHttp
-    from landscape.client.snap.http import SnapdHttpException
-
-    snap_http = SnapHttp()
-
 
 class SnapMonitor(DataWatcher):
 
@@ -37,7 +30,7 @@ def get_data(self):
         for i in range(len(snaps)):
             try:
                 config = snap_http.get_conf(snaps[i]["name"])
-            except SnapdHttpException as e:
+            except SnapdHttpException:
                 config = {}
 
             snaps[i]["config"] = config

diff --git a/landscape/client/monitor/tests/test_snapmonitor.py b/landscape/client/monitor/tests/test_snapmonitor.py
@@ -1,15 +1,11 @@
 from unittest.mock import patch
 
 from landscape.client.monitor.snapmonitor import SnapMonitor
+from landscape.client.snap_http import SnapdHttpException
+from landscape.client.snap_http import SnapdResponse
 from landscape.client.tests.helpers import LandscapeTest
 from landscape.client.tests.helpers import MonitorHelper
 
-try:
-    from snap_http import SnapdHttpException
-    from snap_http import SnapdResponse
-except ImportError:
-    from landscape.client.snap.http import SnapdHttpException
-
 
 class SnapMonitorTest(LandscapeTest):
     """Snap plugin tests."""

diff --git a/landscape/client/snap/__init__.py b/landscape/client/snap/__init__.py