Skip to content

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

Merged
jleveque merged 2 commits intosonic-net:masterfrom
jleveque:remove_encode
Nov 22, 2020
Merged

Remove unnecessary calls to str.encode() now that the package is Python 3; Fix deprecation warning#1260
jleveque merged 2 commits intosonic-net:masterfrom
jleveque:remove_encode

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 requested a review from lguohan November 21, 2020 19:48
@jleveque jleveque self-assigned this Nov 21, 2020
@jleveque jleveque requested a review from qiluo-msft November 21, 2020 19:51
@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