fix(snap): handle revision as str (#92)

Snap revisions can be strings and should be handled as strings

lib/charms/operator_libs_linux/v1/snap.py → lib/charms/operator_libs_linux/v2/snap.py

@@ -79,11 +79,11 @@
 LIBID = "05394e5893f94f2d90feb7cbe6b633cd"
 
 # Increment this major API version when introducing breaking changes
-LIBAPI = 1
+LIBAPI = 2
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 12
+LIBPATCH = 0
 
 
 # Regex to locate 7-bit C1 ANSI sequences
@@ -222,7 +222,7 @@ def __init__(
         name,
         state: SnapState,
         channel: str,
-        revision: int,
+        revision: str,
         confinement: str,
         apps: Optional[List[Dict[str, str]]] = None,
         cohort: Optional[str] = "",
@@ -427,7 +427,7 @@ def _install(
         self,
         channel: Optional[str] = "",
         cohort: Optional[str] = "",
-        revision: Optional[int] = None,
+        revision: Optional[str] = None,
     ) -> None:
         """Add a snap to the system.
 
@@ -454,7 +454,7 @@ def _refresh(
         self,
         channel: Optional[str] = "",
         cohort: Optional[str] = "",
-        revision: Optional[int] = None,
+        revision: Optional[str] = None,
         leave_cohort: Optional[bool] = False,
     ) -> None:
         """Refresh a snap.
@@ -498,7 +498,7 @@ def ensure(
         classic: Optional[bool] = False,
         channel: Optional[str] = "",
         cohort: Optional[str] = "",
-        revision: Optional[int] = None,
+        revision: Optional[str] = None,
     ):
         """Ensure that a snap is in a given state.
 
@@ -575,7 +575,7 @@ def state(self, state: SnapState) -> None:
         self._state = state
 
     @property
-    def revision(self) -> int:
+    def revision(self) -> str:
         """Returns the revision for a snap."""
         return self._revision
 
@@ -828,7 +828,7 @@ def _load_installed_snaps(self) -> None:
                 name=i["name"],
                 state=SnapState.Latest,
                 channel=i["channel"],
-                revision=int(i["revision"]),
+                revision=i["revision"],
                 confinement=i["confinement"],
                 apps=i.get("apps", None),
             )
@@ -846,7 +846,7 @@ def _load_info(self, name) -> Snap:
             name=info["name"],
             state=SnapState.Available,
             channel=info["channel"],
-            revision=int(info["revision"]),
+            revision=info["revision"],
             confinement=info["confinement"],
             apps=None,
         )
@@ -859,7 +859,7 @@ def add(
     channel: Optional[str] = "",
     classic: Optional[bool] = False,
     cohort: Optional[str] = "",
-    revision: Optional[int] = None,
+    revision: Optional[str] = None,
 ) -> Union[Snap, List[Snap]]:
     """Add a snap to the system.
 
@@ -871,7 +871,7 @@ def add(
         classic: an (Optional) boolean specifying whether it should be added with classic
             confinement. Default `False`
         cohort: an (Optional) string specifying the snap cohort to use
-        revision: an (Optional) integer specifying the snap revision to use
+        revision: an (Optional) string specifying the snap revision to use
 
     Raises:
         SnapError if some snaps failed to install or were not found.
@@ -947,7 +947,7 @@ def _wrap_snap_operations(
     channel: str,
     classic: bool,
     cohort: Optional[str] = "",
-    revision: Optional[int] = None,
+    revision: Optional[str] = None,
 ) -> Union[Snap, List[Snap]]:
     """Wrap common operations for bare commands."""
     snaps = {"success": [], "failed": []}

tests/integration/test_snap.py

@@ -9,7 +9,7 @@
 from subprocess import CalledProcessError, check_output, run
 
 import pytest
-from charms.operator_libs_linux.v1 import snap
+from charms.operator_libs_linux.v2 import snap
 from helpers import get_command_path
 
 logger = logging.getLogger(__name__)
@@ -119,7 +119,7 @@ def test_snap_ensure_revision():
         match = re.search(r"latest/edge.*\((\d+)\)", line)
 
         if match:
-            edge_revision = int(match.group(1))
+            edge_revision = match.group(1)
             break
     assert edge_revision is not None
 
@@ -139,7 +139,7 @@ def test_snap_ensure_revision():
             match = re.search(r"installed.*\((\d+)\)", line)
 
             assert match is not None
-            assert int(match.group(1)) == edge_revision
+            assert match.group(1) == edge_revision
 
 
 def test_snap_start():

tests/unit/test_snap.py

@@ -8,9 +8,9 @@
 from unittest.mock import MagicMock, mock_open, patch
 
 import fake_snapd as fake_snapd
-from charms.operator_libs_linux.v1 import snap
+from charms.operator_libs_linux.v2 import snap
 
-patch("charms.operator_libs_linux.v1.snap._cache_init", lambda x: x).start()
+patch("charms.operator_libs_linux.v2.snap._cache_init", lambda x: x).start()
 
 lazy_load_result = r"""
 {
@@ -223,7 +223,7 @@ def test_can_lazy_load_snap_info(self, mock_exists, m):
         self.assertEqual(result.state, snap.SnapState.Available)
         self.assertEqual(result.channel, "stable")
         self.assertEqual(result.confinement, "strict")
-        self.assertEqual(result.revision, 233)
+        self.assertEqual(result.revision, "233")
 
     @patch("os.path.isfile")
     def test_can_load_installed_snap_info(self, mock_exists):
@@ -240,7 +240,7 @@ def test_can_load_installed_snap_info(self, mock_exists):
         self.assertEqual(s["charmcraft"].state, snap.SnapState.Latest)
         self.assertEqual(s["charmcraft"].channel, "latest/stable")
         self.assertEqual(s["charmcraft"].confinement, "classic")
-        self.assertEqual(s["charmcraft"].revision, 603)
+        self.assertEqual(s["charmcraft"].revision, "603")
 
     @patch("os.path.isfile")
     def test_raises_error_if_snap_not_running(self, mock_exists):
@@ -251,15 +251,15 @@ def test_raises_error_if_snap_not_running(self, mock_exists):
         )
         with self.assertRaises(snap.SnapAPIError) as ctx:
             s._load_installed_snaps()
-        self.assertEqual("<charms.operator_libs_linux.v1.snap.SnapAPIError>", ctx.exception.name)
+        self.assertEqual("<charms.operator_libs_linux.v2.snap.SnapAPIError>", ctx.exception.name)
         self.assertIn("snapd is not running", ctx.exception.message)
 
     def test_can_compare_snap_equality(self):
         foo1 = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic")
         foo2 = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic")
         self.assertEqual(foo1, foo2)
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output")
     def test_can_run_snap_commands(self, mock_subprocess):
         mock_subprocess.return_value = 0
         foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic")
@@ -290,7 +290,7 @@ def test_can_run_snap_commands(self, mock_subprocess):
             ["snap", "install", "foo", "--classic", '--revision="123"'], universal_newlines=True
         )
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.run")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.run")
     def test_can_run_snap_daemon_commands(self, mock_subprocess):
         mock_subprocess.return_value = MagicMock()
         foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic")
@@ -319,7 +319,7 @@ def test_can_run_snap_daemon_commands(self, mock_subprocess):
             capture_output=True,
         )
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.run")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.run")
     def test_snap_connect(self, mock_subprocess):
         mock_subprocess.return_value = MagicMock()
         foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic")
@@ -348,7 +348,7 @@ def test_snap_connect(self, mock_subprocess):
             capture_output=True,
         )
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output")
     def test_snap_hold_timedelta(self, mock_subprocess):
         mock_subprocess.return_value = 0
         foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic")
@@ -364,7 +364,7 @@ def test_snap_hold_timedelta(self, mock_subprocess):
             universal_newlines=True,
         )
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output")
     def test_snap_hold_forever(self, mock_subprocess):
         mock_subprocess.return_value = 0
         foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic")
@@ -380,7 +380,7 @@ def test_snap_hold_forever(self, mock_subprocess):
             universal_newlines=True,
         )
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output")
     def test_snap_unhold(self, mock_subprocess):
         mock_subprocess.return_value = 0
         foo = snap.Snap("foo", snap.SnapState.Latest, "stable", "1", "classic")
@@ -396,7 +396,7 @@ def test_snap_unhold(self, mock_subprocess):
             universal_newlines=True,
         )
 
-    @patch("charms.operator_libs_linux.v1.snap.SnapClient.get_installed_snap_apps")
+    @patch("charms.operator_libs_linux.v2.snap.SnapClient.get_installed_snap_apps")
     def test_apps_property(self, patched):
         s = SnapCacheTester()
         s._snap_client.get_installed_snaps.return_value = json.loads(installed_result)["result"]
@@ -406,7 +406,7 @@ def test_apps_property(self, patched):
         self.assertEqual(len(s["charmcraft"].apps), 2)
         self.assertIn({"snap": "charmcraft", "name": "charmcraft"}, s["charmcraft"].apps)
 
-    @patch("charms.operator_libs_linux.v1.snap.SnapClient.get_installed_snap_apps")
+    @patch("charms.operator_libs_linux.v2.snap.SnapClient.get_installed_snap_apps")
     def test_services_property(self, patched):
         s = SnapCacheTester()
         s._snap_client.get_installed_snaps.return_value = json.loads(installed_result)["result"]
@@ -464,7 +464,7 @@ def setUp(self, mock_exists, m):
         snap._Cache.cache._load_installed_snaps()
         snap._Cache.cache._load_available_snaps()
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output")
     def test_can_run_bare_changes(self, mock_subprocess):
         mock_subprocess.return_value = 0
         foo = snap.add("curl", classic=True, channel="latest")
@@ -484,7 +484,7 @@ def test_can_run_bare_changes(self, mock_subprocess):
         )
         self.assertTrue(baz.present)
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess")
     def test_cohort(self, mock_subprocess):
         mock_subprocess.check_output = MagicMock()
 
@@ -512,7 +512,7 @@ def test_cohort(self, mock_subprocess):
             universal_newlines=True,
         )
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output")
     def test_can_ensure_states(self, mock_subprocess):
         mock_subprocess.return_value = 0
         foo = snap.ensure("curl", "latest", classic=True, channel="latest/test")
@@ -533,7 +533,7 @@ def test_can_ensure_states(self, mock_subprocess):
         )
         self.assertTrue(baz.present)
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output")
     def test_raises_snap_not_found_error(self, mock_subprocess):
         def raise_error(cmd, **kwargs):
             # If we can't find the snap, we should raise a CalledProcessError.
@@ -544,10 +544,10 @@ def raise_error(cmd, **kwargs):
         mock_subprocess.side_effect = raise_error
         with self.assertRaises(snap.SnapError) as ctx:
             snap.add("nothere")
-        self.assertEqual("<charms.operator_libs_linux.v1.snap.SnapError>", ctx.exception.name)
+        self.assertEqual("<charms.operator_libs_linux.v2.snap.SnapError>", ctx.exception.name)
         self.assertIn("Failed to install or refresh snap(s): nothere", ctx.exception.message)
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output")
     def test_snap_set(self, mock_subprocess):
         foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "classic")
 
@@ -572,15 +572,15 @@ def test_snap_set(self, mock_subprocess):
                 universal_newlines=True,
             )
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.check_call")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_call")
     def test_system_set(self, mock_subprocess):
         snap._system_set("refresh.hold", "foobar")
         mock_subprocess.assert_called_with(
             ["snap", "set", "system", "refresh.hold=foobar"],
             universal_newlines=True,
         )
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.check_call")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_call")
     def test_system_set_fail(self, mock_subprocess):
         mock_subprocess.side_effect = CalledProcessError(1, "foobar")
         with self.assertRaises(snap.SnapError):
@@ -598,15 +598,15 @@ def test_hold_refresh_invalid_non_bool(self):
         with self.assertRaises(TypeError):
             snap.hold_refresh(forever="foobar")
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.check_call")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_call")
     def test_hold_refresh_reset(self, mock_subprocess):
         snap.hold_refresh(days=0)
         mock_subprocess.assert_called_with(
             ["snap", "set", "system", "refresh.hold="],
             universal_newlines=True,
         )
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.check_call")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_call")
     def test_hold_refresh_forever(self, mock_subprocess):
         snap.hold_refresh(forever=True)
 
@@ -615,8 +615,8 @@ def test_hold_refresh_forever(self, mock_subprocess):
             universal_newlines=True,
         )
 
-    @patch("charms.operator_libs_linux.v1.snap.datetime")
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.check_call")
+    @patch("charms.operator_libs_linux.v2.snap.datetime")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_call")
     def test_hold_refresh_valid_days(self, mock_subprocess, mock_datetime):
         # A little too closely-tied to the internals of hold_refresh(), but at least
         # the test runs whatever your local time zone is.
@@ -638,7 +638,7 @@ def test_ansi_filter(self):
         assert snap.ansi_filter.sub("", "\x1b[0m\x1b[?25h\x1b[Kpypi-server") == "pypi-server"
         assert snap.ansi_filter.sub("", "\x1b[0m\x1b[?25h\x1b[Kparca") == "parca"
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output")
     def test_install_local(self, mock_subprocess):
         mock_subprocess.return_value = "curl XXX installed"
         snap.install_local("./curl.snap")
@@ -647,7 +647,7 @@ def test_install_local(self, mock_subprocess):
             universal_newlines=True,
         )
 
-    @patch("charms.operator_libs_linux.v1.snap.subprocess.check_output")
+    @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output")
     def test_install_local_args(self, mock_subprocess):
         mock_subprocess.return_value = "curl XXX installed"
         for kwargs, cmd_args in [