Re: Multi-series charm authors: new functionality for comparing ubuntu versions

2017-04-14 Thread Alex Kavanagh
On Thu, Apr 13, 2017 at 10:13 PM, Colin Watson  wrote:

> On Thu, Apr 13, 2017 at 09:15:08PM +0100, Alex Kavanagh wrote:
> > So instead of:
> >
> > if ubuntu_version > 'trusty':
> >
> > We do:
> >
> > cmp_version = CompareHostReleases(ubuntu_version)
> > if cmp_version > 'trusty':
> >
> > This version of the code checks that ubuntu_version and 'trusty' strings
> > are known releases and makes it easy to fix existing code.  >, <, >=< <=,
> > ==, != are all supported.
>
> I have déjà vu: I did exactly the same thing in lp:ubuntu-cdimage in
> 2012. :-)  I agree that this style makes things pretty nice for calling
> code.
>
> The one problem I've had with it is that every so often somebody looks
> at a bit of code and says "hey, isn't this going to break after 17.04?"
> and then I have to explain that it's actually magically OK, which as
> problems go is not really a bad one to have.
>

Good to know that I'm not totally going out on a limb with this approach!
There was some criticism of leaving is as >, <, etc., the alternatives just
seemed really clunky.  There is the downside that people could miss it, but
there wasn't a reasonable way of stopping people just using a regular
string comparison.  Thanks for the feedback / confirmation!


>
> --
> Colin Watson   [cjwat...@ubuntu.com]
>
> --
> Juju mailing list
> Juju@lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/
> mailman/listinfo/juju
>



-- 
Alex Kavanagh - Software Engineer
Cloud Dev Ops - Solutions & Product Engineering - Canonical Ltd
-- 
Juju mailing list
Juju@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju


Re: Multi-series charm authors: new functionality for comparing ubuntu versions

2017-04-13 Thread Colin Watson
On Thu, Apr 13, 2017 at 09:15:08PM +0100, Alex Kavanagh wrote:
> So instead of:
> 
> if ubuntu_version > 'trusty':
> 
> We do:
> 
> cmp_version = CompareHostReleases(ubuntu_version)
> if cmp_version > 'trusty':
> 
> This version of the code checks that ubuntu_version and 'trusty' strings
> are known releases and makes it easy to fix existing code.  >, <, >=< <=,
> ==, != are all supported.

I have déjà vu: I did exactly the same thing in lp:ubuntu-cdimage in
2012. :-)  I agree that this style makes things pretty nice for calling
code.

The one problem I've had with it is that every so often somebody looks
at a bit of code and says "hey, isn't this going to break after 17.04?"
and then I have to explain that it's actually magically OK, which as
problems go is not really a bad one to have.

-- 
Colin Watson   [cjwat...@ubuntu.com]

-- 
Juju mailing list
Juju@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju


Multi-series charm authors: new functionality for comparing ubuntu versions

2017-04-13 Thread Alex Kavanagh
Hi

I just wanted to highlight some new functionality that has been added to
charm-helpers [1] to make comparing Ubuntu versions more robust.  In the
OpenStack team, all of our charms are 'multi-series' in that a single charm
support multiple versions of Ubuntu and multiple versions of OpenStack
(often simultaneously).

The code was using statements along the lines of:

if ubuntu_version > 'trusty':

The eagle-eyed amongst you will have noticed the impending y2k-type bug,
with the (possible) imminent wrap from 'zesty' -> '???'.

So we decided to fix this across our 30-odd charms and add a couple of
functions to charm-helpers to aid in this.

Firstly, charm-helpers.core.strutils.BasicStringComparator class is an
abstract-ish class to be used as a base class for building a comparator of
an ordered list.  It provides __eq__, __ne__, __lt__ (and so on)
comparisons for two strings.  it also checks that the strings provided are
in the defined list, which helps with mistyping of string literals.

Then for the Ubuntu releases, the
charmhelpers.core.host.CompareHostReleases class is used to compare two
Ubuntu release for equality or 'earlier than' and 'later than;.

So instead of:

if ubuntu_version > 'trusty':

We do:

cmp_version = CompareHostReleases(ubuntu_version)
if cmp_version > 'trusty':

This version of the code checks that ubuntu_version and 'trusty' strings
are known releases and makes it easy to fix existing code.  >, <, >=< <=,
==, != are all supported.

We did consider a number of approaches, but felt that this one was simple
to retro-fit to existing charms and also spot in reviews going forward if
they have been missed.

Also, for charms in the OpenStack ecosystem, there is an additional
comparator class:

charmhelpers.contrib.openstack.utils.CompareOpenStackReleases

which is used in the same way, but works with all of the OpenStack releases.

Thus:

cmp_os_release = CompareOpenStackReleases(os_release)
if cmp_os_release > 'mitaka':

We're now in the process of finalises the fixes to the OpenStack charms and
they'll be in 'next' over the coming week.

Thanks, and hope this is useful.
Best
Alex.


[1] -- charm-helpers: https://launchpad.net/charm-helpers

-- 
Alex Kavanagh - Software Engineer
Cloud Dev Ops - Solutions & Product Engineering - Canonical Ltd
-- 
Juju mailing list
Juju@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju