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

chore: update readme, do not rely on msg being plaintext #1

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

vpavlin
Copy link
Member

@vpavlin vpavlin commented Feb 9, 2024

This PR adds a bit of "how to run the example" into the README and fixes one issue I hit while running the example - when binary (protobuf encoded) messages appear, the py-waku will fail to decode them as utf-8, we should leave decoding to the app (callback) rather than assume plaintext is used

Copy link
Collaborator

@Ivansete-status Ivansete-status left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for it! 💯

I just added some minor comments

NODEKEY=$(openssl rand -hex 32)
```

You car run the `tests/waku_example.py` now as
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tiny typo. But the weird thing is that 'n' and 'r' are not next to each other hehe xD

Suggested change
You car run the `tests/waku_example.py` now as
You can run the `tests/waku_example.py` now as

try:
msgPlain = msg.decode('utf-8')
except:
msgPlain = "[binary content]"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we could append somehow the exception detail

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, the exception is something like "Failed to decode because...", but it is not really useful because we know what the problem is - the msg is in binary format, not plaintext utf-8, so I just wanted to give that note to the user, but the main point is that the message got delivered

README.md Show resolved Hide resolved
@vpavlin vpavlin merged commit 532f548 into waku-org:master Feb 9, 2024
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.

None yet

2 participants