Skip to content

Comments

Fix hostname type checking for fanout devices for new sonic-mgmt docker images#1027

Open
eyakubch wants to merge 1 commit intoAzure:202506from
eyakubch:fix/facts-cache-fanout-hostname
Open

Fix hostname type checking for fanout devices for new sonic-mgmt docker images#1027
eyakubch wants to merge 1 commit intoAzure:202506from
eyakubch:fix/facts-cache-fanout-hostname

Conversation

@eyakubch
Copy link

Problem

With latest sonic-mgmt docker image when run regression we are facing with such error

 def _get_default_zone(function, func_args, func_kargs):
        """
            Default zone getter used for decorator cached.
            For multi asic platforms some the facts will have the namespace to get the facts for an ASIC.
            Add the namespace to the default zone.
        """
        hostname = None
        unicode_type = str if sys.version_info.major >= 3 else unicode      # noqa: F821
        if func_args:
            hostname = getattr(func_args[0], "hostname", None)
        if not hostname or type(hostname) not in [str, unicode_type]:
>           raise ValueError("Failed to get attribute 'hostname' of type string from instance of type %s."
                             % type(func_args[0]))
E           ValueError: Failed to get attribute 'hostname' of type string from instance of type <class 'tests.common.devices.sonic.SonicHost'>.


common/cache/facts_cache.py:209: ValueError

Solution

The fix is present in public sonic-mgmt:tests/common/cache/facts_cache.py‎
https://github.com/sonic-net/sonic-mgmt/pull/21729/changes#diff-96c915e519e2a2620baa4625d4d65a21a8dfc46423c1334d39be729bd3c60e56

Need to port this changes to msft-sonic-mgmt

This fixes the issue where type(hostname) not in [str, unicode_type] fails because type() doesn't handle inheritance properly, while isinstance(hostname, (str, unicode_type)) works correctly across Python versions .

Signed-off-by: Eduard Yakubchyk <eyakubch@cisco.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant