feat(package): Add option to output search results as raw logs. #641
Conversation
WalkthroughThe pull request modifies the Changes
Possibly Related PRs
Suggested Reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
components/clp-package-utils/clp_package_utils/scripts/search.py (1)
125-126: Consider adding a test for the--rawargument handling.While the command construction looks correct, we should ensure proper testing coverage for this new feature.
Would you like me to help create a test case for the
--rawargument handling?components/clp-package-utils/clp_package_utils/scripts/native/search.py (2)
86-110: Excellent refactoring of the worker connection handler, eh!The implementation properly handles raw output formatting through a closure pattern. However, there are a few suggestions for improvement:
- Consider adding type hints for better code maintainability
- The error handling could be more specific than catching all exceptions
Here's a suggested improvement:
-def get_worker_connection_handler(raw_output: bool): +def get_worker_connection_handler(raw_output: bool) -> asyncio.Protocol: async def worker_connection_handler(reader: asyncio.StreamReader, writer: asyncio.StreamWriter): try: unpacker = msgpack.Unpacker() while True: buf = await reader.read(1024) if b"" == buf: return unpacker.feed(buf) for unpacked in unpacker: if raw_output: print(f"{unpacked[1]}", end="") else: print(f"{unpacked[2]}: {unpacked[1]}", end="") - except asyncio.CancelledError: + except (asyncio.CancelledError, msgpack.UnpackException) as e: + logger.error(f"Error processing message: {e}") return finally: writer.close() return worker_connection_handler
98-104: Consider adding buffering for output performance.The current implementation prints each message individually, which might not be optimal for performance with large volumes of logs.
Consider buffering the output:
+ buffer = [] + BUFFER_SIZE = 1000 for unpacked in unpacker: if raw_output: - print(f"{unpacked[1]}", end="") + buffer.append(f"{unpacked[1]}") else: - print(f"{unpacked[2]}: {unpacked[1]}", end="") + buffer.append(f"{unpacked[2]}: {unpacked[1]}") + if len(buffer) >= BUFFER_SIZE: + print("".join(buffer), end="") + buffer.clear()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/clp-package-utils/clp_package_utils/scripts/native/search.py(8 hunks)components/clp-package-utils/clp_package_utils/scripts/search.py(3 hunks)
🔇 Additional comments (5)
components/clp-package-utils/clp_package_utils/scripts/search.py (2)
45-45: Help messages look good, eh!
The help messages for time-related arguments are now properly formatted.
Also applies to: 50-50
64-66: LGTM! The new --raw argument is well-defined.
The argument is properly added with appropriate help text and uses the standard store_true action.
components/clp-package-utils/clp_package_utils/scripts/native/search.py (3)
239-239: Help messages look good!
The time-related argument help messages are properly formatted.
Also applies to: 244-244
263-265: LGTM! The --raw argument is consistently defined.
The argument definition matches the wrapper script implementation.
122-122: Function signatures are properly updated.
The raw_output parameter is consistently propagated through all relevant functions.
Let's verify that all functions using raw output are properly updated:
Also applies to: 136-136, 195-195, 207-207, 297-297
✅ Verification successful
Let me gather more context about the usage of raw_output parameter.
Function signatures are properly updated with raw_output parameter
The verification confirms that:
raw_outputparameter is correctly defined in all relevant function signaturesget_worker_connection_handleraccepts and usesraw_outputdo_search_without_aggregationincludesraw_outputin its parameters- The parameter is properly propagated through function calls:
do_searchcallsdo_search_without_aggregationwith the correct parametersget_worker_connection_handleris called withraw_outputin the server setup
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all functions properly handle raw_output parameter
# Search for function definitions and their calls
ast-grep --pattern 'def $FUNC($ARGS) { $$$
$BODY
}'
# Search for any potential missing raw_output parameter
rg -A 2 'def (do_search|do_search_without_aggregation|get_worker_connection_handler)'
Length of output: 1142
Script:
#!/bin/bash
# Search for the full function signatures and their implementations
rg -A 10 'def (do_search|do_search_without_aggregation|get_worker_connection_handler)' components/clp-package-utils/clp_package_utils/scripts/native/search.py
# Search for calls to these functions to verify parameter passing
rg -B 2 -A 2 '(do_search|do_search_without_aggregation|get_worker_connection_handler)\(' components/clp-package-utils/clp_package_utils/scripts/native/search.py
Length of output: 2239
Description
This PR adds the option to output raw logs from the package search scripts instead of the current
PATH:MSGsyntax as requested in #625. This makes it easier to feed the output of search into tools like jq.Validation performed
--rawoption results in outputting raw logs--rawoption retains same behaviour as priorSummary by CodeRabbit
New Features
--rawfor outputting search results as raw logs.Bug Fixes
--begin-timeand--end-timearguments.