-
Notifications
You must be signed in to change notification settings - Fork 99
Fix: propagate custom_headers to Index instances #1172
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
Conversation
WalkthroughCustom headers propagation is implemented throughout the client hierarchy. The Client stores custom headers and passes them to HttpRequests, TaskHandler, and Index instances. Index also receives and propagates custom headers to its internal HttpRequests and TaskHandler. A test verifies Index inherits Client custom headers. Changes
Sequence DiagramsequenceDiagram
participant User
participant Client
participant Index
participant TaskHandler
participant HttpRequests
User->>Client: __init__(url, api_key, custom_headers=X)
Client->>Client: Store custom_headers as _custom_headers
User->>Client: index(uid="movies")
Client->>Index: __init__(config, uid, custom_headers=_custom_headers)
Index->>HttpRequests: __init__(config, custom_headers)
HttpRequests->>HttpRequests: Store custom_headers
Index->>TaskHandler: __init__(config, custom_headers)
TaskHandler->>HttpRequests: __init__(config, custom_headers)
User->>Index: Operations (get, search, etc.)
Index->>HttpRequests: Request with custom_headers included
HttpRequests-->>Index: Response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
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 (3)
tests/client/test_client.py (1)
63-70: LGTM!Good test coverage for the primary use case. The test correctly verifies that custom headers propagate from Client to Index's HttpRequests instance.
Consider adding an assertion for
index.task_handler.http.headersto verify headers also propagate to the TaskHandler within the Index:# Also verify TaskHandler gets the custom headers assert index.task_handler.http.headers.items() >= custom_headers.items()meilisearch/index.py (2)
62-87: LGTM!The
custom_headersparameter is correctly added to__init__and properly propagated to bothHttpRequestsandTaskHandler. This ensures all HTTP calls from an Index instance will include the custom headers.The docstring (lines 71-80) does not document the new
custom_headersparameter. Consider adding:primary_key: Primary-key of the index. + custom_headers: + Custom headers to add when sending data to Meilisearch. """
178-210: LGTM!The static
createmethod correctly acceptscustom_headersand propagates it to theHttpRequestsinstance used for the POST request. This ensures index creation respects custom headers.The docstring for
create(lines 185-204) does not document the newcustom_headersparameter. Consider adding documentation for completeness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
meilisearch/client.py(5 hunks)meilisearch/index.py(4 hunks)meilisearch/task.py(2 hunks)tests/client/test_client.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
meilisearch/task.py (2)
meilisearch/config.py (1)
Config(6-72)meilisearch/_httprequests.py (1)
HttpRequests(19-227)
meilisearch/client.py (3)
meilisearch/_httprequests.py (1)
HttpRequests(19-227)meilisearch/task.py (1)
TaskHandler(14-212)meilisearch/index.py (2)
Index(54-2384)create(179-210)
🔇 Additional comments (6)
meilisearch/task.py (1)
22-28: LGTM!The
custom_headersparameter is properly added with correct typing (Optional[Mapping[str, str]]) and is correctly propagated to theHttpRequestsinstance. The default value ofNonemaintains backward compatibility.meilisearch/client.py (5)
77-82: LGTM!Storing
custom_headersasself._custom_headersis the correct approach for propagating headers to sub-clients. The underscore prefix appropriately indicates this is for internal use. BothHttpRequestsandTaskHandlercorrectly receive the custom headers.
105-105: LGTM!Custom headers are correctly propagated to
Index.create, ensuring the index creation HTTP request includes the custom headers.
152-162: LGTM!Custom headers are correctly propagated to each Index instance created from the response in
get_indexes.
208-208: LGTM!Custom headers are correctly propagated to the Index instance in
get_index, ensuring subsequent operations on the returned Index will include the headers.
245-247: LGTM!Custom headers are correctly propagated to the Index instance in the
indexmethod. This completes the header propagation for all index-related client methods.
Strift
left a comment
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.
Hello @eun2ce and thanks for your contribution 🙌
Let's get this merged!
Related issue
Fixes #1143
What does this PR do?
custom_headerspassed tomeilisearch.Clientare propagated toIndexinstances and task-related HTTP calls.custom_headerson theClient(self._custom_headers) so that it can be reused when instantiating sub-clients.TaskHandlerto acceptcustom_headersand pass them to its internalHttpRequestsinstance.Index:__init__now accepts an optionalcustom_headersparameter and uses it for bothHttpRequestsandTaskHandler.Index.createnow accepts an optionalcustom_headersparameter and forwards it toHttpRequests.Client:self._custom_headerswhen callingIndex.createincreate_index.self._custom_headerswhen creatingIndexinstances inget_indexes,get_index, andindex.test_index_inherits_custom_headers) to verify thatIndex.http.headersincludes the custom headers when they are provided to theClient.PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.