Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[barefoot][platform] platform API 2.0 fixes #6607

Merged
merged 1 commit into from
Jan 30, 2021
Merged

[barefoot][platform] platform API 2.0 fixes #6607

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions platform/barefoot/sonic-platform-modules-bfn-montara/sonic_platform/eeprom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from sonic_eeprom import eeprom_base
from sonic_eeprom import eeprom_tlvinfo

from .platform_thrift_client import thrift_try
from platform_thrift_client import thrift_try
except ImportError as e:
raise ImportError (str(e) + "- required module not found")

Expand Down Expand Up @@ -64,6 +64,11 @@
EEPROM_SYMLINK = "/var/run/platform/eeprom/syseeprom"
EEPROM_STATUS = "/var/run/platform/eeprom/status"

try:
_str_type = basestring
except NameError:
_str_type = str
Comment on lines +67 to +70
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All applications which import sonic-platform should now use the Python 3 API if it is available (as of a couple weeks ago). Therefore, you should be able stop building/installing a Python 2 sonic-platform package and you could then remove this check, as the string type will always be str with Python 3.

If you'd prefer to keep this for now and leave the remove of removal of Python 2 support for a separate PR, that's OK. Just let me know.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I will prepare a separate PR for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing. I just wanted to prevent you from adding code if not necessary. We can merge this as-is and you can clean it up with the subsequent PR, once you're able to prove you no longer need to build/install a Python 2 package.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for info!


class Eeprom(eeprom_tlvinfo.TlvInfoDecoder):
def __init__(self):

Expand Down Expand Up @@ -101,15 +106,15 @@ def eeprom_parse(self):
f.close()

eeprom_params = ""
for attr, val in self.eeprom.__dict__.iteritems():
for attr, val in self.eeprom.__dict__.items():
if val is None:
continue

elem = eeprom_default_dict.get(attr)
if elem is None:
continue

if isinstance(val, basestring):
if isinstance(val, _str_type):
value = val.replace('\0', '')
else:
value = str(val)
Expand Down