Code review comment for ~jslarraz/ubuntu-security-tools:changelog_fix

Revision history for this message
Steve Beattie (sbeattie) wrote :

On Sun, Oct 29, 2023 at 11:40:30PM -0000, Alex Murray wrote:
> Review: Approve
>
> LGTM - one suggestion to make it more future-proof which would be worthwhile I think.
>
> Diff comments:
>
> > diff --git a/package-tools/tests/test_increment_version.py b/package-tools/tests/test_increment_version.py
> > index 49dfa86..4d75802 100755
> > --- a/package-tools/tests/test_increment_version.py
> > +++ b/package-tools/tests/test_increment_version.py
> > @@ -3,15 +3,20 @@ from increment_version import increment_version_security
> >
> >
> > class TestIncrementVersion(unittest.TestCase):
> > +
> > + def setUp(self):
> > + self.devel_release = "noble"
>
> Can we instead do something like the following to make this more future-proof:
>
> self.devel_release = subprocess.check_output(["ubuntu-distro-info", "--devel"]) or "noble"

There are two issues with this approach, one fixable, one not:

1) using a subprocess unnecessarily, when the distro_info python module
   is already present:

   $ python3 -q
   >>> import distro_info
   >>> ubuntu_info = distro_info.UbuntuDistroInfo()
   >>> ubuntu_info.devel()
   'noble'

2) there is a small window of time between the last devel releasse
   (e.g. mantic) gets released and when the new release has not
   been announced where the distro-info-data package does not have
   information about the current development release, like on one of
   my systems that has not pulled in the updated package:

   $ distro-info --devel
   ubuntu-distro-info: Distribution data outdated.
   Please check for an update for distro-info-data. See /usr/share/doc/distro-info-data/README.Debian for details.
   $ python3 -q
   >>> import distro_info
   >>> ubuntu_info = distro_info.UbuntuDistroInfo()
   >>> ubuntu_info.devel()
   Traceback (most recent call last):
     File "<stdin>", line 1, in <module>
     File "/usr/lib/python3/dist-packages/distro_info.py", line 163, in devel
       raise DistroDataOutdated()
   distro_info.DistroDataOutdated: Distribution data outdated. Please check for an update for distro-info-data. See /usr/share/doc/distro-info-data/README.Debian for details.

I also dislike having another place to edit in the ReleaseCycle tasks,
but distro-info-data's unreliability kind of makes that difficult to
deal with, so we may just need to put up with it.

Most tools in the security-tools repo deal with releases via
~/.ubuntu-security-tools.conf to be independent of UCT's knowledge
of releases I guess, which we would definitely want for something we
might run in a CI pipeline, but that also means we wouldn't have a
configured security-tools.conf file to read, either.

Thanks.

--
Steve Beattie
<email address hidden>

« Back to merge proposal