#122 Create a tool to update short latest symlinks automatically
Merged by lsedlar. Opened by ounsal.
ounsal/compose-utils master  into  master

JIRA: RHELCMP-4378

Signed-off-by: Ozan Unsal ounsal@redhat.com

This default doesn't seem to be helpful. It install RPM it will point to /usr/bin. I think either current working directory or no default at all would be better.

The sort considers 1.2.0 to be higher than 1.10.0. It's not a common case, but can happen :)

It works as expected. Few more comments:

  • Would it be possible to generalize to any number of version components? Now it only works with versions X.Y.Z. With a different number, strange things happen.

  • Can you move the code into a module (either a new one or symlink.py) so that tests can be written for it?

This default doesn't seem to be helpful. It install RPM it will point to /usr/bin. I think either current working directory or no default at all would be better.

Done

The sort considers 1.2.0 to be higher than 1.10.0. It's not a common case, but can happen :)

Done

It works as expected. Few more comments:
Would it be possible to generalize to any number of version components? Now it only works with versions X.Y.Z. With a different number, strange things happen.
Can you move the code into a module (either a new one or symlink.py) so that tests can be written for it?

Done

rebased onto fdbc9f91549fcff9bb690a4f83eacb2aefac0291

rebased onto 45baa3c830557cb62c48d028edc5d1d647c58483

rebased onto 7d1b095a2cb0e15788777fb34c6b9aedb6399f14

'(\d+)' should either be a raw string or the backslash should be escaped.

Unlike the previous version, this will have all symlinks pointing to the full one (even latest-X-1 will point to latest-X-1.0.0 rather than latest-X-1.0). I actually like that, it means one less step in getting to the actual data.

I'd like to see a few more test cases though. Something like this:

diff --git a/tests/test_symlink.py b/tests/test_symlink.py
index 832a997..b3227d5 100644
--- a/tests/test_symlink.py
+++ b/tests/test_symlink.py
@@ -124,9 +124,43 @@ class TestLatestSymlink(unittest.TestCase):
             symlink.print_latest_link(path, version_type=symlink.VersionType.MINOR)
         self.assertEqual(mock_out.getvalue(), 'latest-DP-1.0\n')
-    def test_update_symlinks(self):
-        path = self._write_compose()
-        symlink.create_latest_link(path, version_type=symlink.VersionType.FULL)
-        path = os.path.join(path, "..")
-        symlink.update_symlinks(path)
-        self.assertTrue(os.path.islink(os.path.join(path, 'latest-DP-1')))
+
+class TestUpdateSymlinks(unittest.TestCase):
+    def setUp(self):
+        self.compose_dir = tempfile.mkdtemp()
+
+    def tearDown(self):
+        shutil.rmtree(self.compose_dir)
+
+    def create_full_symlink(self, compose_id):
+        os.symlink('not-needed', os.path.join(self.compose_dir, compose_id))
+
+    def assert_symlink(self, name, dest):
+        link = os.path.join(self.compose_dir, name)
+        self.assertTrue(os.path.islink(link))
+        self.assertEqual(os.readlink(link), dest)
+
+    def test_two_component(self):
+        self.create_full_symlink('latest-DP-1.0')
+
+        symlink.update_symlinks(self.compose_dir)
+
+        self.assert_symlink('latest-DP-1', 'latest-DP-1.0')
+
+    def test_three_component(self):
+        self.create_full_symlink('latest-DP-1.0.0')
+
+        symlink.update_symlinks(self.compose_dir)
+
+        self.assert_symlink('latest-DP-1', 'latest-DP-1.0.0')
+        self.assert_symlink('latest-DP-1.0', 'latest-DP-1.0.0')
+
+    def test_scenario(self):
+        for y in range(1, 4):
+            self.create_full_symlink('latest-DP-1.%d.0' % y)
+            symlink.update_symlinks(self.compose_dir)
+
+        self.assert_symlink('latest-DP-1', 'latest-DP-1.3.0')
+        self.assert_symlink('latest-DP-1.1', 'latest-DP-1.1.0')
+        self.assert_symlink('latest-DP-1.2', 'latest-DP-1.2.0')
+        self.assert_symlink('latest-DP-1.3', 'latest-DP-1.3.0')

A few more boring details: in order for the file to be installed, it needs to be listed in setup.py. Can you also create a man page for it? Copy an existing one, tweak as needed, preview with man ./docs/compose-update-latest-symlinks.1.

The script crashes when path is not provided. Either make it a required argument, or use . as default.

rebased onto 11e26f89073560f1c7e8c381d63bd2eec0d7d268

'(\d+)' should either be a raw string or the backslash should be escaped.

Done

Unlike the previous version, this will have all symlinks pointing to the full one (even latest-X-1 will point to latest-X-1.0.0 rather than latest-X-1.0). I actually like that, it means one less step in getting to the actual data.

Done

A few more boring details: in order for the file to be installed, it needs to be listed in setup.py. Can you also create a man page for it? Copy an existing one, tweak as needed, preview with man ./docs/compose-update-latest-symlinks.1.

Done

The script crashes when path is not provided. Either make it a required argument, or use . as default.

Done

rebased onto f00dce842565498ea6cdb69dd316f06350577f66

Looks good to me. Merging :tada:

Pull-Request has been merged by lsedlar