[syseepromd] Support both new platform API and old platform plugins

[stephenxs]

Support new platform api with plugin mode compatible.
In this PR, new platform api is supported in following steps:

1. initialization, load new-platform-api-based class eeprom or chassis for syseepromd and psud respectively, if failed then load old style plugin.
2. for each api, call new platform api, old style plugin is called if the former raised a NotImplementError; other Exceptions raised will be passed to caller without handling.

now only two APIs are called in syseepromd, update_eeprom_db and read_eeprom.
it depends on the sonic-platform-common #48 add get_eeprom to Chassis for syseepromd to access

[jleveque]

After thinking about this more, I think we can make this change even smaller. Because no matter which API gets loaded (the new API or the old plugin), the resulting object is handled the same way. Do you see any reason why we can't do the following:

1. Remove eeprom_util, load_eeprom_util(), load_platform_api() and all _wrapper_*() functions completely.
2. Replace lines 136-140 with something like the following:

        # First, try to load the new platform API
        try:
            import sonic_platform
            self.chassis = sonic_platform.platform.Platform().get_chassis()
            self.eeprom = self.chassis.get_eeprom()
        except Exception as e:
            logger.log_warning("Failed to load platform-specific eeprom from sonic_platform package due to {}".format(repr(e)))
        finally:
            # If we didn't successfully load the class from the sonic_platform package, try loading the old plugin
            if not self.eeprom:
                try:
                    self.eeprom = self.load_platform_util(PLATFORM_SPECIFIC_MODULE_NAME, PLATFORM_SPECIFIC_CLASS_NAME)
                except Exception as e:
                    logger.log_error("Failed to load platform-specific eeprom implementation: %s" % (str(e)), True)

        if not self.eeprom:
            sys.exit(ERR_EEPROMUTIL_LOAD)

Then all the functions can simply reference self.eeprom. Will this work or am I missing something?

[stephenxs]

Hi Joe, Can it be well handled if another exception thrown from the finally block? I’m not sure whether it’s a good style. Other thing seems good to me. BTW, psud can also be updated in this way. But for xcvrd, it is a little complicated, because the sfputil has to be remained even new platform api supported. I’ve mentioned that in another PR #38

[jleveque]

The finally block is a bit of a messy approach, I agree. Instead, you should be able to simply check for None:

        # First, try to load the new platform API
        try:
            import sonic_platform
            self.chassis = sonic_platform.platform.Platform().get_chassis()
            self.eeprom = self.chassis.get_eeprom()
        except Exception as e:
            logger.log_warning("Failed to load platform-specific eeprom from sonic_platform package due to {}".format(repr(e)))

        # If we didn't successfully load the class from the sonic_platform package, try loading the old plugin
        if not self.eeprom:
            try:
                self.eeprom = self.load_platform_util(PLATFORM_SPECIFIC_MODULE_NAME, PLATFORM_SPECIFIC_CLASS_NAME)
            except Exception as e:
                logger.log_error("Failed to load platform-specific eeprom implementation: %s" % (str(e)), True)

        if not self.eeprom:
            sys.exit(ERR_EEPROMUTIL_LOAD)

Great to know psud should be able to use the same approach. I assumed xcvrd would require a different solution.

[stephenxs]

Eepromd works in this way. But I don’t think we should remove the _wrapper_* functions if we want to have a per-API level compat. PSU doesn’t work in this way, since the it requires both chassis and psu object, unlike eeprom which requires eeprom object only. The root cause is that, 1. there can be more than one PSUs in a switch so we have both APIs to access PSU information on a global scope, like get_num_psus which is provided by chassis, and APIs to access each PSU, like get_psu_presence provided by psu class. 2. there is only one eeprom in a switch and it doesn’t require any “global” scope APIs.

[jleveque]

Understood. So do you want to make any changes to this PR after all?

[stephenxs]

The finally block is a bit of a messy approach, I agree. Instead, you should be able to simply check for None:

        # First, try to load the new platform API
        try:
            import sonic_platform
            self.chassis = sonic_platform.platform.Platform().get_chassis()
            self.eeprom = self.chassis.get_eeprom()
        except Exception as e:
            logger.log_warning("Failed to load platform-specific eeprom from sonic_platform package due to {}".format(repr(e)))

        # If we didn't successfully load the class from the sonic_platform package, try loading the old plugin
        if not self.eeprom:
            try:
                self.eeprom = self.load_platform_util(PLATFORM_SPECIFIC_MODULE_NAME, PLATFORM_SPECIFIC_CLASS_NAME)
            except Exception as e:
                logger.log_error("Failed to load platform-specific eeprom implementation: %s" % (str(e)), True)

        if not self.eeprom:
            sys.exit(ERR_EEPROMUTIL_LOAD)

Great to know psud should be able to use the same approach. I assumed xcvrd would require a different solution.

    # First, try to load the new platform API
    try:
        import sonic_platform
        self.chassis = sonic_platform.platform.Platform().get_chassis()
        self.eeprom = self.chassis.get_eeprom()
    except Exception as e:
        logger.log_warning("Failed to load platform-specific eeprom from sonic_platform package due to {}".format(repr(e)))

    # If we didn't successfully load the class from the sonic_platform package, try loading the old plugin
    if not self.eeprom:
        try:
            self.eeprom = self.load_platform_util(PLATFORM_SPECIFIC_MODULE_NAME, PLATFORM_SPECIFIC_CLASS_NAME)
        except Exception as e:
            logger.log_error("Failed to load platform-specific eeprom implementation: %s" % (str(e)), True)

    if not self.eeprom:
        sys.exit(ERR_EEPROMUTIL_LOAD)

Understood. So do you want to make any changes to this PR after all?

Sure. I'm testing the syseepromd.

[stephenxs]

Understood. So do you want to make any changes to this PR after all?

Hi Joe, syseepromd has been updated. Please review. Thank you.