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

Fix mock SonicV2Connector in python3: use decode_responses mode so caller code will be the same as python2 #1238

Merged
merged 1 commit into from
Nov 14, 2020

Conversation

qiluo-msft
Copy link
Contributor

The python2 behavior is not changed.

- What I did

- How I did it

- How to verify it

- Previous command output (if the output of a command-line utility has changed)

- New command output (if the output of a command-line utility has changed)

Copy link
Contributor

@jleveque jleveque left a comment

Choose a reason for hiding this comment

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

Looks good. Tested with Python 3.

@@ -86,6 +95,7 @@ def __init__(self, *args, **kwargs):
topo = kwargs.pop('topo')
namespace = kwargs.pop('namespace')
db_name = kwargs.pop('db_name')
self.decode_responses = kwargs.pop('decode_responses', False) == True
Copy link
Contributor

Choose a reason for hiding this comment

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

is == True necessary here?

Copy link
Contributor

@jleveque jleveque Nov 13, 2020

Choose a reason for hiding this comment

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

I asked this same question on Qi's earlier similar PR for swsssdk :)

It normalizes decode_responses to always be a Bool. E.g., if kwargs.pop('decode_responses', False) returns None or some other unexpected value. It's basically a failsafe.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, so this is similar to !!<bool> trick in C. It appears to me that is is a bug in the downstream rather that some we should work around it??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no bug in downstream. Here I just program conservatively.

@qiluo-msft qiluo-msft merged commit 8079558 into sonic-net:master Nov 14, 2020
@qiluo-msft qiluo-msft deleted the qiluo/mockdecode branch November 14, 2020 01:04
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.

3 participants