PR#3531: Error on list-tagged --sigs --paths without mount - koji

koji

#3531 Error on list-tagged --sigs --paths without mount

Merged a year ago by tkopecek. Opened 2 years ago by tkopecek.

tkopecek/koji issue934 into master

Error on list-tagged --sigs --paths without mount

Tomas Kopecek • a year ago

0880935

cli/koji_cli/commands.py

file modified

+22 -9

		`@@ -2713,21 +2713,34 @@`
		`if not taginfo:`
		`parser.error("No such tag: %s" % tag)`

		`+ if options.sigs and options.paths:`
		`+ packages_dir = os.path.join(koji.BASEDIR, 'packages')`
		`+ if not os.path.exists(packages_dir):`
		`+ error("'list-tagged --sigs --paths' requires accessible %s" % packages_dir)`
		`+`
		`if options.rpms:`
		`rpms, builds = session.listTaggedRPMS(tag, **opts)`
		`data = rpms`
		`if options.paths:`
		`- build_idx = dict([(b['id'], b) for b in builds])`
		`- for rinfo in data:`
		`- build = build_idx[rinfo['build_id']]`
		`+ build_idx = {}`
		`+ for build in builds:`
		`+ build_idx[build['id']] = build`
		`builddir = pathinfo.build(build)`
		`- if options.sigs:`
		`- sigkey = rinfo['sigkey']`
		`- signedpath = os.path.join(builddir, pathinfo.signed(rinfo, sigkey))`
		`- if os.path.exists(signedpath):`
		`- rinfo['path'] = signedpath`
		`+ if os.path.isdir(builddir):`
		`+ build['_dir'] = builddir`
		`else:`
		`- rinfo['path'] = os.path.join(builddir, pathinfo.rpm(rinfo))`
		`+ warn('Build directory not found: %s' % builddir)`
		`+ for rinfo in data:`
		`+ build = build_idx[rinfo['build_id']]`
		`+ builddir = build.get('_dir')`
		`+ if builddir:`
		`+ if options.sigs:`
		`+ sigkey = rinfo['sigkey']`
		`+ signedpath = os.path.join(builddir, pathinfo.signed(rinfo, sigkey))`
		`+ if os.path.exists(signedpath):`
		`+ rinfo['path'] = signedpath`
		`+ else:`
		`+ rinfo['path'] = os.path.join(builddir, pathinfo.rpm(rinfo))`
		`fmt = "%(path)s"`
		`data = [x for x in data if 'path' in x]`
		`else:`

tests/test_cli/test_list_tagged.py

file modified

+6 -2

		`@@ -122,9 +122,11 @@`
		`self.session.listTagged.assert_not_called()`
		`self.assert_console_message(stdout, expected)`

		`+ @mock.patch('os.path.isdir', return_value=True)`
		`+ @mock.patch('os.path.exists', return_value=True)`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`@mock.patch('koji.util.eventFromOpts', return_value=None)`
		`- def test_list_tagged_rpms_paths(self, event_from_opts_mock, stdout):`
		`+ def test_list_tagged_rpms_paths(self, event_from_opts_mock, stdout, os_path_exists, isdir):`
		`expected = """/mnt/koji/packages/packagename/version/1.el6/noarch/rpmA-0.0.1-1.el6.noarch.rpm`
		`/mnt/koji/packages/packagename/version/1.el6/x86_64/rpmA-0.0.1-1.el6.x86_64.rpm`
		`"""`
		`@@ -138,13 +140,15 @@`
		`self.tag, package=self.pkg, inherit=None, latest=3, arch=['x86_64'])`
		`self.session.listTagged.assert_not_called()`

		`+ @mock.patch('os.path.exists')`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`@mock.patch('koji.util.eventFromOpts', return_value=None)`
		`- def test_list_tagged_sigs_paths(self, event_from_opts_mock, stdout):`
		`+ def test_list_tagged_sigs_paths(self, event_from_opts_mock, stdout, os_path_exists):`
		`expected = ""`
		`args = [self.tag, self.pkg, '--latest-n=3', '--rpms', '--sigs',`
		`'--arch=x86_64', '--paths']`

		`+ os_path_exists.side_effect = [True, False, False]`
		`anon_handle_list_tagged(self.options, self.session, args)`
		`self.assert_console_message(stdout, expected)`
		`self.ensure_connection_mock.assert_called_once_with(self.session, self.options)`

tkopecek commented 2 years ago

mikem commented 2 years ago

We probably need a more involved check for the fs. The /mnt/koji directory could exist and not be correct (e.g. not currently mounted on a client). I would also avoid using the word 'mounted' in the error as there are situations where the base dir exists and is not a mount.

So perhaps something like:

if options.sigs and options.paths:
    packages_dir = os.path.join(koji.BASEDIR, 'packages')
    if not os.path.exists(packages_dir):
        error("'list-tagged --sigs --paths' requires accessible %s" % packages_dir)

Also, in the --sigs --paths case, we could add a further check like:

@@ -2718,11 +2718,18 @@ def anon_handle_list_tagged(goptions, session, args):
         rpms, builds = session.listTaggedRPMS(tag, **opts)
         data = rpms
         if options.paths:
-            build_idx = dict([(b['id'], b) for b in builds])
+            build_idx = {}
+            for build in builds:
+                build_idx[b['id']] = build
+                builddir = pathinfo.build(build)
+                if os.path.isdir(builddir):
+                    build['_dir'] = builddir
+                else:
+                    warn('Build directory not found: %s' % builddir)
             for rinfo in data:
                 build = build_idx[rinfo['build_id']]
-                builddir = pathinfo.build(build)
-                if options.sigs:
+                builddir = build.get('_dir')
+                if options.sigs and builddir:
                     sigkey = rinfo['sigkey']
                     signedpath = os.path.join(builddir, pathinfo.signed(rinfo, sigkey))
                     if os.path.exists(signedpath):

1 new commit added