#817 Update the volume ID substitutions list and application
Merged 6 years ago by mikem. Opened 6 years ago by adamwill.
adamwill/koji update-substitutions  into  master

file modified
+27 -16
@@ -2828,25 +2828,34 @@ 

          return hdrlist

  

      def _shortenVolID(self, name, version, release):

-         # Based on code from pungi

+         # Duplicated with pungi-fedora fedora.conf

          substitutions = {

-             'MATE_Compiz': 'MATE',

-             'Security': 'Sec',

-             'Electronic_Lab': 'Elec',

-             'Robotics': 'Robo',

-             'Scientific_KDE': 'SciK',

-             'Design_suite': 'Dsgn',

-             'Games': 'Game',

-             'Jam_KDE': 'Jam',

-             'Workstation': 'WS',

-             'Server': 'S',

-             'Cloud': 'C',

-             'Alpha': 'A',

-             'Beta': 'B',

-             'TC': 'T',

+                          'Beta': 'B',

+                       'Rawhide': 'rawh',

+                 'Astronomy_KDE': 'AstK',

+                        'Atomic': 'AH',

+                      'Cinnamon': 'Cinn',

+                         'Cloud': 'C',

+                  'Design_suite': 'Dsgn',

+                'Electronic_Lab': 'Elec',

+                    'Everything': 'E',

+                         'Games': 'Game',

+                        'Images': 'img',

+                       'Jam_KDE': 'Jam',

+                   'MATE_Compiz': 'MATE',

+              # Note https://pagure.io/pungi-fedora/issue/533

+              'Python-Classroom': 'Clss',

+              'Python_Classroom': 'Clss',

+                      'Robotics': 'Robo',

+                'Scientific_KDE': 'SciK',

+                      'Security': 'Sec',

+                        'Server': 'S',

+                   'Workstation': 'WS',

+             'WorkstationOstree': 'WS',

          }

  

-         for k, v in substitutions.iteritems():

+         # Duplicated with pungi/util.py _apply_substitutions

+         for k, v in sorted(substitutions.items(), key=lambda x: len(x[0]), reverse=True):

              if k in name:

                  name = name.replace(k, v)

              if k in version:
@@ -2855,6 +2864,8 @@ 

                  release = release.replace(k, v)

  

          volid = "%s-%s-%s" % (name, version, release)

+         # Difference: pungi treats result more than 32 characters long as

+         # fatal and raises an error

          return volid[:32]

  

  

Some newer subvariants / loadouts, and 'Rawhide' for some
reason, were missing from the substitution list. This updates
it, alphabetizes it, adjusts the indentation to match how it
is in pungi-fedora for ease of comparison, and also tweaks the
way the list is applied to match
https://pagure.io/pungi/pull-request/857 , the correct way to
ensure longer substitutions are applied before shorter ones.

Note 'Alpha' and 'TC' are removed because we don't do Alphas or
TCs any more.

Signed-off-by: Adam Williamson awilliam@redhat.com

rebased onto 68ae910

6 years ago

Corresponding pungi-fedora PR here.

Please don't indent like that.

I've never been happy having this code here, as it is highly specific not only to Fedora, but to Fedora versions. Not to mention the duplication.

This part always seemed more like configuration than code. I expect for Pungi too.

"Please don't indent like that."

That's the way it is in pungi-fedora. It's much easier trying to check them against each other if they look the same than if they look different. I figured pungi-fedora is the canonical source, so koji should match how it's done there.

"I've never been happy having this code here, as it is highly specific not only to Fedora, but to Fedora versions. Not to mention the duplication."

I'm not either, but that seems orthogonal to the issue of fixing it for now. It's already there and being used, but because it's missing bits, actual real live images are getting fairly useless, overly-truncated volume IDs. Examining the volume IDs of Rawhide live images is how I noticed the problem in the first place.

I think the reason the duplication exists at all is that koji does not allow Pungi to simply tell it what the volume ID should be, for live images. Or, for that matter, the ISO name. So Koji winds up duplicating the code to produce those things with Pungi. If spin-livemedia allowed Pungi to pass those things in, we could avoid the duplication (and have a much better shot at keeping the format for those things consistent between different images).

"This part always seemed more like configuration than code. I expect for Pungi too."

For Pungi it is configuration - they're defined in fedora.conf, which is a configuration file. (Pungi uses kobo.conf.PyConfigParser, a config parser that allows python-y syntax, which is why this config looks like a Python dict). A different distro can use Pungi with different substitutions (I don't know if RHEL actually uses this mechanism, but it could).

That's the way it is in pungi-fedora. It's much easier trying to check them against each other if they look the same than if they look different. I figured pungi-fedora is the canonical source, so koji should match how it's done there.

I get that. I guess we can keep it that way with a note and eventually add a flake8 exclusion for it.

I think the reason the duplication exists at all is that koji does not allow Pungi to simply tell it what the volume ID should be

I'm not opposed to such an approach. The current implementation was contributed from Fedora. Perhaps Dennis was wanted to ensure that the short name was consistent with the nvr.

For Pungi it is configuration

Ah, ok.

I've filed #833 for follow up

Commit 12b9af5 fixes this pull-request

Pull-Request has been merged by mikem

6 years ago

Thanks, Mike. Believe me I'm not super happy with how this whole mess works either, but making it better seems like a longer-term project that isn't on anyone's immediate radar, and I just wanted to fix the volume IDs for now :/