refactor: move to z85base91 package #62
Conversation
|
Warning Rate limit exceeded@JarbasAl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 48 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis pull request involves a significant refactoring of the encoding modules within the hivemind_bus_client library. The primary change is migrating encoding implementations (Z85B, Z85P, B91) from the current library to an external package called Changes
Assessment against linked issues
Possibly related PRs
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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
hivemind_bus_client/encodings/z85b.py (1)
1-1: Unused importZ85BThe
Z85Bclass is imported but not used within this module. This may lead to unnecessary resource usage and can be safely removed to clean up the code.Apply this diff to remove the unused import:
-from z85base91 import Z85BAlternatively, if you intend to re-export
Z85Bfor backward compatibility, consider adding it to the module's public interface:__all__ = ["Z85B"]🧰 Tools
🪛 Ruff (0.8.2)
1-1:
z85base91.Z85Bimported but unusedRemove unused import:
z85base91.Z85B(F401)
hivemind_bus_client/encodings/z85p.py (1)
1-1: Unused importZ85PThe
Z85Pclass is imported but not used within this module. Removing unused imports helps in maintaining clean code and optimizing resource usage.Apply this diff to remove the unused import:
-from z85base91 import Z85PIf your intention is to re-export
Z85Pfor backward compatibility, consider adding it to the module's public interface:__all__ = ["Z85P"]🧰 Tools
🪛 Ruff (0.8.2)
1-1:
z85base91.Z85Pimported but unusedRemove unused import:
z85base91.Z85P(F401)
hivemind_bus_client/z85b.py (1)
3-3: Unused importZ85BThe
Z85Bclass is imported but not utilized within this module. It's advisable to remove unused imports to keep the codebase clean and efficient.Apply this diff to remove the unused import:
-from z85base91 import Z85BAlternatively, if you intend to re-export
Z85Bfor backward compatibility, you can expose it through the module's public interface:__all__ = ["Z85B"]Or assign it directly for re-export:
Z85B = Z85B🧰 Tools
🪛 Ruff (0.8.2)
3-3:
z85base91.Z85Bimported but unusedRemove unused import:
z85base91.Z85B(F401)
hivemind_bus_client/encodings/b91.py (1)
1-10: Consider adding migration guide documentation.This refactoring to use the optimized C implementation in
z85base91is a good architectural decision. To ensure a smooth transition for users:
- Consider adding a migration guide in the README or documentation
- Include code examples showing how to update from the old to new package
- Document any performance improvements users can expect
🧰 Tools
🪛 Ruff (0.8.2)
1-1:
z85base91.B91imported but unusedRemove unused import:
z85base91.B91(F401)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.github/workflows/unit_tests.yml(0 hunks)hivemind_bus_client/encodings/__init__.py(1 hunks)hivemind_bus_client/encodings/b91.py(1 hunks)hivemind_bus_client/encodings/benchmark.py(2 hunks)hivemind_bus_client/encodings/z85b.py(1 hunks)hivemind_bus_client/encodings/z85p.py(1 hunks)hivemind_bus_client/encryption.py(1 hunks)hivemind_bus_client/http_client.py(4 hunks)hivemind_bus_client/z85b.py(1 hunks)requirements.txt(1 hunks)test/requirements.txt(0 hunks)test/test_b91.py(0 hunks)test/test_z85b.py(0 hunks)test/test_z85p.py(0 hunks)
💤 Files with no reviewable changes (5)
- test/requirements.txt
- test/test_b91.py
- .github/workflows/unit_tests.yml
- test/test_z85b.py
- test/test_z85p.py
✅ Files skipped from review due to trivial changes (1)
- hivemind_bus_client/encryption.py
🧰 Additional context used
🪛 Ruff (0.8.2)
hivemind_bus_client/z85b.py
3-3: z85base91.Z85B imported but unused
Remove unused import: z85base91.Z85B
(F401)
hivemind_bus_client/encodings/z85b.py
1-1: z85base91.Z85B imported but unused
Remove unused import: z85base91.Z85B
(F401)
hivemind_bus_client/encodings/__init__.py
1-1: z85base91.Z85B imported but unused
(F401)
1-1: z85base91.Z85P imported but unused
(F401)
1-1: z85base91.B91 imported but unused
(F401)
hivemind_bus_client/encodings/z85p.py
1-1: z85base91.Z85P imported but unused
Remove unused import: z85base91.Z85P
(F401)
hivemind_bus_client/encodings/b91.py
1-1: z85base91.B91 imported but unused
Remove unused import: z85base91.B91
(F401)
🔇 Additional comments (12)
hivemind_bus_client/encodings/z85b.py (1)
5-10: Deprecation warning is properly implementedThe deprecation warning correctly informs users about the deprecation and advises them to use the new package
z85base91. The use ofstacklevel=2ensures that the warning points to the user's code, which is good practice.hivemind_bus_client/encodings/z85p.py (1)
5-10: Deprecation warning is properly implementedThe deprecation warning appropriately notifies users about the deprecation and guides them to use the new package
z85base91. Includingstacklevel=2in the warning ensures that the warning points to the user's code.hivemind_bus_client/z85b.py (1)
8-8: Deprecation warning message updated correctlyThe deprecation warning message has been updated to inform users about the deprecation and advise them to use the new package
z85base91. This helps in a smooth transition for users relying on the old import path.hivemind_bus_client/encodings/__init__.py (2)
1-1: LGTM! Import statement updated correctly.The encoders are now imported from the z85base91 package as intended.
🧰 Tools
🪛 Ruff (0.8.2)
1-1:
z85base91.Z85Bimported but unused(F401)
1-1:
z85base91.Z85Pimported but unused(F401)
1-1:
z85base91.B91imported but unused(F401)
5-10: Well-structured deprecation warning.The warning effectively communicates the deprecation and provides clear guidance for users to update their code.
hivemind_bus_client/encodings/benchmark.py (2)
182-182: Increased iterations for more accurate benchmarking.Changing default iterations from 20 to 1000 will provide more statistically significant results.
266-276: Review benchmark results showing performance tradeoffs.The benchmark results show:
- JSON-B64 variants lead in performance but have higher size overhead
- z85base91 implementations (B91, Z85B, Z85P) offer better bandwidth efficiency but lower performance
- This aligns with the PR objective of providing optimized implementations with different tradeoffs
hivemind_bus_client/http_client.py (3)
76-76: Good addition of connection state management.Using threading.Event for connection state tracking improves thread safety and robustness.
257-258: Connection state validation added consistently.Connection checks are properly added to emit, get_messages, and get_binary_messages methods with clear error messages.
Also applies to: 350-351, 360-361
337-337: TODO comment needs implementation.The TODO comment indicates that the connection state is set without validating the POST request response.
Would you like me to provide an implementation for proper connection state validation based on the response?
hivemind_bus_client/encodings/b91.py (2)
4-10: Well-implemented deprecation warning!The deprecation warning is clear, informative, and follows best practices:
- Clearly indicates what's deprecated
- Provides migration instructions
- Uses correct warning category
- Sets appropriate stacklevel
1-2: Verify the purpose of importing B91.While static analysis flags
B91as unused, this import might be necessary for maintaining backward compatibility during the transition period. Please confirm if this is intentional.Run this script to check if this module is re-exporting
B91:✅ Verification successful
The B91 import is correctly used as part of the deprecation strategy
The import is intentional and serves as part of the transition to the
z85base91package. The module re-exportsB91through__init__.pywhile warning users to update their imports. This maintains backward compatibility for existing code.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if B91 is re-exported in __init__.py rg -l "from.*b91.*import.*B91" "hivemind_bus_client/encodings/__init__.py" # Check for any direct imports of B91 from this module rg -l "from.*hivemind_bus_client.encodings.b91.*import.*B91"Length of output: 137
Script:
#!/bin/bash # View complete content of b91.py cat hivemind_bus_client/encodings/b91.py # Search for any B91-related patterns rg "B91" --type py # Check __init__.py content for any re-exports cat hivemind_bus_client/encodings/__init__.pyLength of output: 2084
🧰 Tools
🪛 Ruff (0.8.2)
1-1:
z85base91.B91imported but unusedRemove unused import:
z85base91.B91(F401)
encoders moved to standalone package with optimized C implementation https://github.com/JarbasHiveMind/z85base91
closes #59
Summary by CodeRabbit
Dependency Changes
z85base91packagepy-cpuinfoEncoding Module Updates
z85base91packageWorkflow Changes
HTTP Client Updates
HiveMindHTTPClientTest Files
B91,Z85B, andZ85Pencodings