#938 Deprecating list-tag-history and tagHistory
Merged 2 years ago by tkopecek. Opened 4 years ago by breilly.
breilly/koji dep-lth-836  into  master

file modified
+7 -4
@@ -1608,10 +1608,10 @@ 

          is_protected = False

          last_latest = None

          tags = {}

-         for entry in session.tagHistory(build=binfo['id']):

-             #we used tagHistory rather than listTags so we can consider tags

+         for entry in session.queryHistory(build=binfo['id'])['tag_listing']:

+             #we used queryHistory rather than listTags so we can consider tags

              #that the build was recently untagged from

-             tags.setdefault(entry['tag_name'], 1)

+             tags.setdefault(entry['tag.name'], 1)

          if options.debug:

              print("Tags: %s" % list(tags.keys()))

          for tag_name in tags:
@@ -1630,7 +1630,7 @@ 

                  continue

              #in order to determine how recently this build was latest, we have

              #to look at the tagging history.

-             hist = session.tagHistory(tag=tag_name, package=binfo['name'])

+             hist = session.queryHistory(tag=tag_name, package=binfo['name'])['tag_listing']

              if not hist:

                  #really shouldn't happen

                  raise koji.GenericError("No history found for %s in %s" % (nvr, tag_name))
@@ -3886,6 +3886,9 @@ 

      parser.add_option("--tag", help=_("Only show data for a specific tag"))

      parser.add_option("--all", action="store_true", help=_("Allows listing the entire global history"))

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

+     koji.util.deprecated("list-tag-history is deprecated and will be removed in a future version. "

+                          "See: https://pagure.io/koji/issue/836")

+ 

      if len(args) != 0:

          parser.error(_("This command takes no arguments"))

          assert False  # pragma: no cover

file modified
+4
@@ -6845,7 +6845,11 @@ 

      package: only for given package

      build: only for given build

      tag: only for given tag

+ 

+     Deprecated; will be removed in a future version

+     See: https://pagure.io/koji/issue/836

      """

+     logger.warning("The tag_history call is deprecated and will be removed in a future version.")

      fields = ('build.id', 'package.name', 'build.version', 'build.release',

                'tag.id', 'tag.name', 'tag_listing.active',

                'tag_listing.create_event', 'tag_listing.revoke_event',

file modified
+4 -4
@@ -486,7 +486,7 @@ 

              if age < min_age:

                  continue

          #see how long build has been untagged

-         history = session.tagHistory(build=binfo['id'])

+         history = session.queryHistory(build=binfo['id'])['tag_listing']

          age = None

          binfo2 = None

          if not history:
@@ -632,7 +632,7 @@ 

              # skip the rest when salvaging

              continue

          # determine how long this build has been in the trashcan

-         history = session.tagHistory(build=binfo['id'], tag=trashcan_tag)

+         history = session.queryHistory(build=binfo['id'], tag=trashcan_tag)['tag_listing']

          current = [x for x in history if x['active']]

          if not current:

              #untagged just now?
@@ -819,14 +819,14 @@ 

          if options.debug:

              print("Pruning tag: %s" % tagname)

          #get builds

-         history = session.tagHistory(tag=tagname, active=True, queryOpts={'order': '-create_ts'})

+         history = session.queryHistory(tag=tagname, active=True)['tag_listing']

          if not history:

              if options.debug:

                  print("No history for %s" % tagname)

              continue

          pkghist = {}

          for h in history:

-             if taginfo['maven_include_all'] and h['maven_build_id']:

+             if taginfo['maven_include_all'] and h.get('maven_build_id', default=None):

                  pkghist.setdefault(h['name'] + '-' + h['version'], []).append(h)

              else:

                  pkghist.setdefault(h['name'], []).append(h)

no initial comment

Unsure if
h.get('maven_build_id', default=None)
will be sufficient to catch maven builds - will queryHistory automatically pull in that field for maven builds, or should something be added to the queryHistory call?

maven_build_id is never populated by queryHistory

This bit:

         #get builds
-        history = session.tagHistory(tag=tagname, active=True, queryOpts={'order': '-create_ts'})
+        history = session.queryHistory(tag=tagname, active=True)['tag_listing']
         if not history:

queryHistory doesn't support queryOpts and doesn't sort the results. Perhaps it should do one or both of these. Until it does, perhaps this bit should do the sorting itself. Or perhaps this is an indication that we should hold of on this deprecation for a release or two in order to establish feature parity with the old call.

@mikem @tkopecek did we ever establish feature parity with the old call?

Yes, we're using new one everywhere (tagHistory is subset of what queryHistory does).

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

2 years ago

Metadata Update from @jcupova:
- Pull-request tagged with: testing-done

2 years ago

Commit cb8d096 fixes this pull-request

Pull-Request has been merged by tkopecek

2 years ago

This PR was merged without addressing the concern I raised above.

queryHistory doesn't support queryOpts and doesn't sort the results

It still does not, and koji-gc could now be using the wrong order when deciding which builds to prune

I have filed #2270 to follow up on that