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

[docker-database] Fix Python3 issue #7700

Merged
merged 2 commits into from
May 31, 2021

Conversation

msosyak
Copy link
Contributor

@msosyak msosyak commented May 25, 2021

Why I did it

To avoid the following error

Traceback (most recent call last):
  File "/usr/local/bin/flush_unused_database", line 10, in <module>
    if 'PONG' in output:
TypeError: a bytes-like object is required, not 'str'

communicate method returns the strings if streams were opened in text mode; otherwise, bytes.
In our case text arg in Popen is not true and that means that communicate return the bytes

How I did it

Set text=True to get strings instead of bytes

How to verify it

run /usr/local/bin/flush_unused_database inside database container

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

@msosyak msosyak requested a review from lguohan as a code owner May 25, 2021 08:23
@msosyak
Copy link
Contributor Author

msosyak commented May 25, 2021

@qiluo-msft I have missed one more fix in the previous PR #7658. Check it, please.

@lguohan lguohan requested a review from qiluo-msft May 26, 2021 15:31
@@ -6,7 +6,7 @@ import time

while(True):
output = subprocess.Popen(['sonic-db-cli', 'PING'], stdout=subprocess.PIPE).communicate()[0]
Copy link
Collaborator

@qiluo-msft qiluo-msft May 26, 2021

Choose a reason for hiding this comment

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

Popen

Thanks for the fix! I see the text=True could also fix this, and it is more readable. Could you instead use that optional parameter? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Description updated

@qiluo-msft qiluo-msft merged commit 3bf60b3 into sonic-net:master May 31, 2021
qiluo-msft pushed a commit that referenced this pull request Jun 2, 2021
#### Why I did it
To avoid the following error
```
Traceback (most recent call last):
  File "/usr/local/bin/flush_unused_database", line 10, in <module>
    if 'PONG' in output:
TypeError: a bytes-like object is required, not 'str'
```
`communicate` method returns the strings if streams were opened in text mode; otherwise, bytes.
In our case text arg  in Popen is not true and that means that `communicate` return the bytes
#### How I did it
Set `text=True` to get strings instead of bytes
#### How to verify it
run `/usr/local/bin/flush_unused_database` inside database container
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
#### Why I did it
To avoid the following error
```
Traceback (most recent call last):
  File "/usr/local/bin/flush_unused_database", line 10, in <module>
    if 'PONG' in output:
TypeError: a bytes-like object is required, not 'str'
```
`communicate` method returns the strings if streams were opened in text mode; otherwise, bytes.
In our case text arg  in Popen is not true and that means that `communicate` return the bytes
#### How I did it
Set `text=True` to get strings instead of bytes
#### How to verify it
run `/usr/local/bin/flush_unused_database` inside database container
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