#2979 Add a utility function to watch builds
Opened 9 months ago by oturpe. Modified 5 months ago
oturpe/koji watch-builds-binding  into  master

file modified
+7 -34
@@ -44,6 +44,7 @@ 

      print_task_recurse,

      unique_path,

      warn,

+     watch_builds,

      watch_logs,

      watch_tasks,

      truncate_string
@@ -7339,8 +7340,6 @@ 

                             "value only")

      (suboptions, args) = parser.parse_args(args)

  

-     start = time.time()

- 

      builds = [koji.parse_NVR(build) for build in suboptions.builds]

      if len(args) < 1:

          parser.error("Please specify a tag name")
@@ -7382,38 +7381,12 @@ 

                  warn("nvr %s is not current in tag %s\n  latest build in %s is %s" %

                       (expected_nvr, tag, tag, present_nvr))

  

-     last_repo = None

-     repo = session.getRepo(tag_id)

- 

-     while True:

-         if builds and repo and repo != last_repo:

-             if koji.util.checkForBuilds(session, tag_id, builds, repo['create_event'],

-                                         latest=True):

-                 if not suboptions.quiet:

-                     print("Successfully waited %s for %s to appear in the %s repo" %

-                           (koji.util.duration(start), koji.util.printList(suboptions.builds), tag))

-                 return

- 

-         if (time.time() - start) >= (suboptions.timeout * 60.0):

-             if not suboptions.quiet:

-                 if builds:

-                     error("Unsuccessfully waited %s for %s to appear in the %s repo" %

-                           (koji.util.duration(start), koji.util.printList(suboptions.builds), tag))

-                 else:

-                     error("Unsuccessfully waited %s for a new %s repo" %

-                           (koji.util.duration(start), tag))

-             error()

- 

-         time.sleep(options.poll_interval)

-         last_repo = repo

-         repo = session.getRepo(tag_id)

- 

-         if not builds:

-             if repo != last_repo:

-                 if not suboptions.quiet:

-                     print("Successfully waited %s for a new %s repo" %

-                           (koji.util.duration(start), tag))

-                 return

+     try:

+         watch_builds(

+             session, tag_id, builds, quiet=suboptions.quiet,

+             poll_interval=options.poll_interval, timeout=suboptions.timeout)

+     except koji.GenericError as e:

+         error(str(e))

(IOW, regarding my previous comment, it would be better to always include the explanatory text in the exception, and change the call here to be something like...)

try:
    watch_builds(...)
except koji.GenericError as e:
    error(str(e) if not suboptions.quiet else "")

  

  

  def handle_regen_repo(options, session, args):

file modified
+57
@@ -819,6 +819,63 @@ 

      return tasklist

  

  

+ def watch_builds(session, tag_id, builds, quiet=False, poll_interval=5, timeout=120):

+     """Watch for given builds to appear in given tag. If no build are given,

+     watch for new repo for given tag.

+ 

+     :param session: Koji session object

+     :param tag_id: Tag id

+     :param builds: List of builds as NVR dicts

+     :param quiet: no/verbose

+     :param poll_interval: Poll interval in seconds

+     :param timeout: Watch timeout in minutes"""

+     last_repo = None

+     repo = session.getRepo(tag_id)

+ 

+     # String representations for logs and exceptions

+     builds_str = koji.util.printList(

+         [koji.buildLabel(build) for build in builds])

+     tag_info = session.getTag(tag_id)

+     if not tag_info:

+         raise koji.GenericError("No tag with id: %s" % tag_id)

+     tag_name = tag_info['name']

+ 

+     start = time.time()

+     while True:

+         if builds and repo and repo != last_repo:

+             if koji.util.checkForBuilds(session, tag_id, builds,

+                                         repo['create_event'], latest=True):

+                 if not quiet:

+                     print("Successfully waited %s for %s "

Since the stated goal here is to make watch_build available to external tool implementations like bodhi, etc., would it make more sense to have quiet=True as the default, so that callers won't be surprised by status messages on the terminal unless they explicitly set quiet=True when calling watch_build()?

+                           "to appear in the %s repo" %

+                           (koji.util.duration(start), builds_str, tag_name))

+                 return

+ 

+         if (time.time() - start >= timeout * 60.0):

+             if not quiet:

+                 if builds:

+                     raise koji.GenericError(

+                         "Unsuccessfully waited %s for %s "

+                         "to appear in the %s repo" %

+                         (koji.util.duration(start), builds_str, tag_name))

+                 else:

+                     raise koji.GenericError(

+                         "Unsuccessfully waited %s for a new %s repo" %

+                         (koji.util.duration(start), tag_name))

+             raise koji.GenericError()

Having the quiet argument control the text content of the exception raised is... very weird. I would think that the exception should always be raised with all appropriate detail; the caller can determine whether or not to output that content based on their quiet status. (Even better would be to have more precise exceptions defined, where this could be e.g. a koji.TimeoutError derived from koji.GenericError. That way the caller can handle the different failure conditions differently.)

+ 

+         time.sleep(poll_interval)

+         last_repo = repo

+         repo = session.getRepo(tag_id)

+ 

+         if not builds:

+             if repo != last_repo:

+                 if not quiet:

+                     print("Successfully waited %s for a new %s repo" %

+                           (koji.util.duration(start), tag_name))

+                 return

+ 

+ 

  def format_inheritance_flags(parent):

      """Return a human readable string of inheritance flags"""

      flags = ''

@@ -0,0 +1,72 @@ 

+ from __future__ import absolute_import

+ import mock

+ import os

+ import rpm

+ import unittest

+ 

+ import koji

+ 

+ class TestBuildLabel(unittest.TestCase):

+     def test_buildLabel(self):

+         """Test the buildLabel method"""

+ 

+         self.assertRaises(AttributeError, koji.buildLabel, None)

+         self.assertRaises(AttributeError, koji.buildLabel, 1)

+         self.assertRaises(AttributeError, koji.buildLabel, [])

+ 

+         input = {}

+         ret = koji.buildLabel(input)

+         self.assertEqual(ret, "None-None-None")

+ 

+         input = {"name": "foo"}

+         ret = koji.buildLabel(input)

+         self.assertEqual(ret, "foo-None-None")

+ 

+         input = {"version": "1.0.2"}

+         ret = koji.buildLabel(input)

+         self.assertEqual(ret, "None-1.0.2-None")

+ 

+         input = {"release": "2"}

+         ret = koji.buildLabel(input)

+         self.assertEqual(ret, "None-None-2")

+ 

+         input = {"name": "foo", "version": "1.0.2"}

+         ret = koji.buildLabel(input)

+         self.assertEqual(ret, "foo-1.0.2-None")

+ 

+         input = {"name": "foo", "release": "2"}

+         ret = koji.buildLabel(input)

+         self.assertEqual(ret, "foo-None-2")

+ 

+         input = {"version": "1.0.2", "release": "2"}

+         ret = koji.buildLabel(input)

+         self.assertEqual(ret, "None-1.0.2-2")

+ 

+         input = {"name": "foo", "version": "1.0.2", "release": "2"}

+         ret = koji.buildLabel(input)

+         self.assertEqual(ret, "foo-1.0.2-2")

+ 

+         input = {"package_name": "bar", "version": "1.0.2", "release": "2"}

+         ret = koji.buildLabel(input)

+         self.assertEqual(ret, "bar-1.0.2-2")

+ 

+         input = {

+             "package_name": "bar",

+             "name": "foo",

+             "version": "1.0.2",

+             "release": "2"

+         }

+         ret = koji.buildLabel(input)

+         self.assertEqual(ret, "bar-1.0.2-2")

+ 

+         input = {"epoch": 7, "name": "foo", "version": "1.0.2", "release": "2"}

+         ret = koji.buildLabel(input)

+         self.assertEqual(ret, "foo-1.0.2-2")

+ 

+         input = {"epoch": 7, "name": "foo", "version": "1.0.2", "release": "2"}

+         ret = koji.buildLabel(input, True)

+         self.assertEqual(ret, "7:foo-1.0.2-2")

+ 

+         input = {"name": "foo", "version": "1.0.2", "release": "2"}

+         ret = koji.buildLabel(input, True)

+         self.assertEqual(ret, "foo-1.0.2-2")

External tools that interact with Koji
need to wait for builds to appear in a repo.
For example, Fedora cli tools 'bodhi' and 'fedpkg'
can create buildroot overrides,
which only become useful
after the override's build appears in the correct repo.
At the moment, 'bodhi' waits for a repo
by invoking the 'koji wait-repo' cli tool
while 'fedpkg' does not wait at all.
In order to make it easier for such tools to wait for a repo,
the wait implementation from 'koji wait-repo'
is moved to koji.utils namespace
where it is available for use through Python import.

Tests for the new function watchBuilds are still missing.
I intend to add them,
but since this my first contribution to Koji,
I would like to get a round of review first,
so my approach is validated
and I will not end up spending time in
writing tests for a feature that will not be merged.

buildLabel does this (with option to not/show epoch) Also note that epoch is integer, not string.

This is not the right library - watch_builds is CLI thing, so it should be in the CLI lib. Moving to koji_cli/lib.py is ok (it is also importable)

buildLabel does this (with option to not/show epoch) Also note that epoch is integer, not string.

Nice, I was looking for existing implementation,
for some reason I did not spot buildLabel.
I will use that instead.

Now that I already wrote some unit tests,
and buildLabel apparently did not have any,
I will move the new tests to test buildLabel.

rebased onto c940e6b

6 months ago

Since the stated goal here is to make watch_build available to external tool implementations like bodhi, etc., would it make more sense to have quiet=True as the default, so that callers won't be surprised by status messages on the terminal unless they explicitly set quiet=True when calling watch_build()?

Having the quiet argument control the text content of the exception raised is... very weird. I would think that the exception should always be raised with all appropriate detail; the caller can determine whether or not to output that content based on their quiet status. (Even better would be to have more precise exceptions defined, where this could be e.g. a koji.TimeoutError derived from koji.GenericError. That way the caller can handle the different failure conditions differently.)

(IOW, regarding my previous comment, it would be better to always include the explanatory text in the exception, and change the call here to be something like...)

try:
    watch_builds(...)
except koji.GenericError as e:
    error(str(e) if not suboptions.quiet else "")