[virt-tools-list] [virt-manager PATCH v2 1/3] osdict: expand the "unknown" check for any distro

[Pavel Hrdina]

On Fri, Oct 19, 2018 at 06:54:52PM +0200, Fabiano Fidêncio wrote:
> Currently osinfo-db has "unknown" entries for fedora, opensuse and
> asianux. Considering this list may grow even more at some point, let's
> just make the check more generic and use it for all of them instead of

s/thos/those/

> keeping it for fedora only.
> 
> Changes have also been done in urldetect and tests_url, as thos also
> used latest_fedora_version().
> 
> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
> ---
>  tests/test_urls.py    |  4 ++--
>  virtinst/osdict.py    | 22 +++++++++++++++-------
>  virtinst/urldetect.py |  2 +-
>  3 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/test_urls.py b/tests/test_urls.py
> index 098c5996..e7ab7142 100644
> --- a/tests/test_urls.py
> +++ b/tests/test_urls.py
> @@ -130,11 +130,11 @@ def _sanitize_osdict_name(detectdistro):
>      if detectdistro == "testsuite-fedora-rawhide":
>          # Special value we use in the test suite to always return the latest
>          # fedora when checking rawhide URL
> -        return OSDB.latest_fedora_version()
> +        return OSDB.latest_os_version("fedora")
>  
>      if re.match("fedora[0-9]+", detectdistro):
>          if not OSDB.lookup_os(detectdistro):
> -            ret = OSDB.latest_fedora_version()
> +            ret = OSDB.latest_os_version("fedora")
>              print("\nConverting detectdistro=%s to latest value=%s" %
>                      (detectdistro, ret))
>              return ret

This 'if' is no longer part of virt-manager code-base.

> diff --git a/virtinst/osdict.py b/virtinst/osdict.py
> index 28e076da..49085e6d 100644
> --- a/virtinst/osdict.py
> +++ b/virtinst/osdict.py
> @@ -208,11 +208,15 @@ class _OSDB(object):
>              return None
>  
>          osname = ret[0].get_short_id()
> -        if osname == "fedora-unknown":
> -            osname = self.latest_fedora_version()
> -            logging.debug("Detected location=%s as os=fedora-unknown. "
> -                "Converting that to the latest fedora OS version=%s",
> -                location, osname)
> +        if osname.endswith("-unknown"):
> +            # We want to get whatever the name is apart from unknown, instead
> +            # of getting the distro, as some enterprise distros may have more
> +            # than one "unknown" entry and we'd like to match the correct one.
> +            osdistro = osname.split("-unknown")[0]
> +            osname = self.latest_os_version(osdistro)
> +            logging.debug("Detected location=%s as os=%s-unknown. "
> +                "Converting that to the latest %s OS version=%s",
> +                osdistro, osdistro, location, osname)

Doesn't look correct.  It should be:

logging.debug("Detected location='%s' as os='%s-unknown'. "
    "Converting that to the latest '%s' OS version='%s'.",
    location, osdistro, osdistro, osname)

>          return osname
>  
> @@ -236,8 +240,12 @@ class _OSDB(object):
>              return None
>          return oses[0]
>  
> -    def latest_fedora_version(self):
> -        return self.latest_regex("fedora[0-9]+")
> +    def latest_os_version(self, osdistro):
> +        regex = osdistro + "[0-9]+"
> +        if osdistro[-1].isdigit():
> +            regex = osdistro + r"\.[0-9]+"
> +
> +        return self.latest_regex(regex)

How about this:

    def latest_os_version(self, osdistro):
        version = r"\.[0-9]+" if osdistro[-1].isdigit() else "[0-9]+"
        return self.latest_regex(osdistro + version)

It's more Pythonic and will create less objects for GC to cleanup.

In C you would write:

    version = condition ? exp1 : exp2;

in python it's a bit different:

    version = exp1 if condition else exp2

Otherwise looks good, I can fix these minor issues before pushing.

Pavel

>  
>  
>  #####################
> diff --git a/virtinst/urldetect.py b/virtinst/urldetect.py
> index 5da15d0b..37d5caf1 100644
> --- a/virtinst/urldetect.py
> +++ b/virtinst/urldetect.py
> @@ -412,7 +412,7 @@ class FedoraDistro(RedHatDistro):
>          return cache.treeinfo_family_regex(famregex)
>  
>      def _detect_version(self):
> -        latest_variant = OSDB.latest_fedora_version()
> +        latest_variant = OSDB.latest_os_version("fedora")
>  
>          verstr = self.cache.treeinfo_version
>          if not verstr:
> -- 
> 2.19.1
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20181116/d3e1a56a/attachment.sig>