-
-
Notifications
You must be signed in to change notification settings - Fork 4
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
performance: reduce_payload_size #46
Conversation
change encoding of encrypted payloads to reduce message size
change encoding of encrypted payloads to reduce message size
change encoding of encrypted payloads to reduce message size
WalkthroughThe pull request focuses on enhancing the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
hivemind_bus_client/util.py (5)
105-118
: New 'b64' parameter and fallback approach.
Allowing the caller to toggle between hex and base64 is good for backwards compatibility. Document the plan to defaultb64=True
in the next major release.
129-129
: Spare blank line.
Consider removing if not needed, to keep readability consistent.- +
133-137
: Streamline the assignment ofdecoder
using a ternary operator.
This simplifies the control flow and is suggested by static analysis.-def decode(b64=False): - if b64: - decoder = pybase64.b64decode - else: - decoder = unhexlify +def decode(b64=False): + decoder = pybase64.b64decode if b64 else unhexlify🧰 Tools
🪛 Ruff (0.8.2)
133-136: Use ternary operator
decoder = pybase64.b64decode if b64 else unhexlify
instead ofif
-else
-blockReplace
if
-else
-block withdecoder = pybase64.b64decode if b64 else unhexlify
(SIM108)
222-222
: Minor spacing.
No functional impact; keep or remove for clarity.
224-237
: Informative main block for local testing.
Demonstrates how to useencrypt_as_json
anddecrypt_from_json
with both b64 and hex encoding. Consider moving these tests into formal unit tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hivemind_bus_client/util.py
(10 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
hivemind_bus_client/util.py
133-136: Use ternary operator decoder = pybase64.b64decode if b64 else unhexlify
instead of if
-else
-block
Replace if
-else
-block with decoder = pybase64.b64decode if b64 else unhexlify
(SIM108)
🔇 Additional comments (19)
hivemind_bus_client/util.py (19)
3-4
: Great addition of encoding imports.
These imports (hexlify
, unhexlify
, Union
, and Dict
) will facilitate encoding/decoding functionalities and strongly typed signatures.
6-6
: Adoption of pybase64
noted.
This library provides a performance benefit for base64 operations compared to Python's built-in base64
.
13-13
: Clearer function signature with type hints.
Using Union[HiveMessage, Message, Dict] -> str
helps maintain clarity about expected inputs/outputs.
27-27
: Type annotation increases clarity.
Defining payload: Union[HiveMessage, Message, str] -> Dict
ensures the function's return type is explicit and consistent.
58-58
: Improved function signature.
Accepts varied message formats (HiveMessage
, Message
, str
, Dict
) for seamless handling, returning a normalized dictionary.
71-71
: Conversion to a HiveMessage with type hints.
Ensures consistent transformation to a HiveMessage
object, reducing runtime type errors.
85-85
: Ensuring a Mycroft Message
is returned.
Accepting Union[HiveMessage, str, Dict]
and returning a Message
clarifies the pipeline from various payload formats.
124-124
: Expanded type hints for decrypt function.
Accepting Union[str, bytes]
is more flexible and clarifies usage.
130-132
: Helpful comment clarifying new vs. old encoding style.
Documents hex vs. base64 expectations.
138-145
: Good handling of ciphertext, tag, and nonce.
Proper decoding from user-provided values can avoid mismatched or truncated data.
146-146
: Potential edge cases in uppercase detection.
any(a.isupper() for a in str(data))
might misinterpret strings with uppercase JSON keys. If that's truly your detection method, consider clarifying or using metadata.
147-157
: Robust fallback approach for decoding.
Attempting base64 decoding if the initial attempt fails can help older clients with mismatched encoding. Good user experience.
160-160
: Enhanced type annotations for encrypt_bin
.
Strengthens clarity by allowing both str
or bytes
.
171-171
: Consistent key trimming logic for decrypt_bin
.
Matches the approach used in encrypt_bin
.
186-186
: compress_payload
with typed input.
Ensuring a str
or bytes
is compressed properly improves consistency.
195-195
: decompress_payload
now returns bytes
.
Clearer function contract helps manage subsequent transformations.
197-202
: Dynamic base64 vs. hex decoding.
Automatically inferring the encoding from uppercase presence is practical, but make sure it doesn't conflict with normal uppercase in hex strings.
206-206
: cast2bytes
usage clarified.
Correctly converts dictionaries and strings, with optional compression.
217-217
: bytes2str
with optional decompression.
Ensures consistent round-trip with cast2bytes
.
change encoding of encrypted payloads to reduce message size
Summary by CodeRabbit
Improvements
Bug Fixes