added the logs api#760
Open
valkrypton wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (Rust)
participant Reqwest as Reqwest HTTP Client
participant Meili as Meilisearch API
participant Network as Network Stream
Client->>Reqwest: open_log_stream(LogStreamRequest)
Reqwest->>Reqwest: serialize JSON, set Content-Type: application/json
Reqwest->>Meili: POST /logs/stream
Meili->>Network: accept streaming connection
Network-->>Reqwest: streaming response (bytes)
Reqwest-->>Client: Stream<Item = Result<Bytes>>
Client->>Client: iterate/process chunks
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
valkrypton
force-pushed
the
add/logs-api
branch
from
248bfc3 to
636b5cc
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.code-samples.meilisearch.yaml:
- Around line 2081-2092: Replace the invalid type and variable names so the
sample compiles: swap the old GetLogs type for the current API name (e.g.,
GetLogsConfig or GetLogsOptions as exported by the crate) and construct
logs_config with that type, and fix the typo by using the byte_stream variable
returned from client.open_log_stream(...) (change byte_s.for_each to
byte_stream.for_each) while keeping the same closure logic; references: GetLogs,
logs_config, open_log_stream, byte_stream, byte_s.
In `@src/logs.rs`:
- Around line 73-86: The test test_open_log_stream creates a hardcoded client
via Client::new("http://localhost:7700", Some("secret")), which bypasses the
meilisearch_test harness; replace that creation with the macro-injected Client
provided by meilisearch_test (i.e., use the Client instance the macro supplies
rather than calling Client::new) so the test uses the harness-managed server;
keep the rest of the test (LogStreamRequest, logs_config, and the
open_log_stream assertion) unchanged.
- Around line 30-70: The open_log_stream method on Client currently starts
streaming with res.bytes_stream() without checking HTTP status; call
.error_for_status_ref()? on the Response returned by
self.http_client.inner().post(...).body(...).send().await to surface non-2xx
errors before converting to a byte stream, then call .bytes_stream() on the
checked Response (ensure you use error_for_status_ref() so the Response is not
consumed), keeping function signature and returning the stream as before.
Comment on lines
+2081
to
+2092
| use futures::StreamExt; | ||
| let client = Client::new(MEILISEARCH_URL, Some(MEILISEARCH_API_KEY)).unwrap(); | ||
| let logs_config = GetLogs { | ||
| target: "info".to_string(), | ||
| mode: LogMode::Human | ||
| }; | ||
| let byte_stream = client.open_log_stream(logs_config).await.unwrap(); | ||
| byte_s.for_each(|chunk| async { | ||
| if let Ok(chunk) = chunk { | ||
| println!("{}", String::from_utf8_lossy(&chunk)); | ||
| } | ||
| }).await; |
Contributor
There was a problem hiding this comment.
Fix type name and variable typo so the sample compiles.
GetLogs doesn’t exist in the new API surface and byte_s is undefined.
🔧 Proposed fix
- let logs_config = GetLogs {
+ let logs_config = LogStreamRequest {
target: "info".to_string(),
mode: LogMode::Human
};
let byte_stream = client.open_log_stream(logs_config).await.unwrap();
- byte_s.for_each(|chunk| async {
+ byte_stream.for_each(|chunk| async {
if let Ok(chunk) = chunk {
println!("{}", String::from_utf8_lossy(&chunk));
}
}).await;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| use futures::StreamExt; | |
| let client = Client::new(MEILISEARCH_URL, Some(MEILISEARCH_API_KEY)).unwrap(); | |
| let logs_config = GetLogs { | |
| target: "info".to_string(), | |
| mode: LogMode::Human | |
| }; | |
| let byte_stream = client.open_log_stream(logs_config).await.unwrap(); | |
| byte_s.for_each(|chunk| async { | |
| if let Ok(chunk) = chunk { | |
| println!("{}", String::from_utf8_lossy(&chunk)); | |
| } | |
| }).await; | |
| use futures::StreamExt; | |
| let client = Client::new(MEILISEARCH_URL, Some(MEILISEARCH_API_KEY)).unwrap(); | |
| let logs_config = LogStreamRequest { | |
| target: "info".to_string(), | |
| mode: LogMode::Human | |
| }; | |
| let byte_stream = client.open_log_stream(logs_config).await.unwrap(); | |
| byte_stream.for_each(|chunk| async { | |
| if let Ok(chunk) = chunk { | |
| println!("{}", String::from_utf8_lossy(&chunk)); | |
| } | |
| }).await; |
🤖 Prompt for AI Agents
In @.code-samples.meilisearch.yaml around lines 2081 - 2092, Replace the invalid
type and variable names so the sample compiles: swap the old GetLogs type for
the current API name (e.g., GetLogsConfig or GetLogsOptions as exported by the
crate) and construct logs_config with that type, and fix the typo by using the
byte_stream variable returned from client.open_log_stream(...) (change
byte_s.for_each to byte_stream.for_each) while keeping the same closure logic;
references: GetLogs, logs_config, open_log_stream, byte_stream, byte_s.
636b5cc to
db4b78c
Compare
Author
|
@Strift can someone review this? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Related issue
Fixes #759
What does this PR do?
logsthat defines theLogModeenum, andLogStreamRequestrequired for the/logs/streamendpoint.NewLogLevelstruct used in the/logs/stderrendpoint to customise the log level.customize_log_levels,interrupt_log_stream, andopen_log_streammethods toClientfor logs related operations.PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.