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.

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

Remove unnecessary calls to str.encode() now that the package is Python 3; Fix deprecation warning #1260

Merged
merged 2 commits into from
Nov 22, 2020

Conversation

jleveque
Copy link
Contributor

@jleveque jleveque commented Nov 21, 2020

In Python 3, all strings are unicode by default. There is no need to encode strings to bytes.

Fixes vsimage check build failure in sonic-net/sonic-buildimage#5926, due to caclmgrd test failing because acl-loader was encoding the table names to bytes and comparing them against strings, thus failing to match table names and therefore failing to load ACL rules.

Also fix Python 3 deprecation warning in decode-syseeprom, as the imp module is deprecated in favor of importlib

@jleveque jleveque self-assigned this Nov 21, 2020
@jleveque jleveque changed the title [acl-loader][fwutil] Remove unnecessary calls to bytes.encode() now that the package is Python 3 [acl-loader][fwutil] Remove unnecessary calls to str.encode() now that the package is Python 3 Nov 21, 2020
@jleveque jleveque changed the title [acl-loader][fwutil] Remove unnecessary calls to str.encode() now that the package is Python 3 Remove unnecessary calls to str.encode() now that the package is Python 3; Fix other compliance issues Nov 21, 2020
@jleveque jleveque changed the title Remove unnecessary calls to str.encode() now that the package is Python 3; Fix other compliance issues Remove unnecessary calls to str.encode() now that the package is Python 3; Fix deprecation warning Nov 21, 2020
@jleveque jleveque merged commit f9eb739 into sonic-net:master Nov 22, 2020
@jleveque jleveque deleted the remove_encode branch November 22, 2020 02:42
@qiluo-msft
Copy link
Contributor

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants