fix: accept imageDownloadHeaders camelCase field name in search() - #1474
Open
mayuriphad wants to merge 1 commit into
Open
fix: accept imageDownloadHeaders camelCase field name in search()#1474mayuriphad wants to merge 1 commit into
mayuriphad wants to merge 1 commit into
Conversation
SearchQuery declares imageDownloadHeaders with alias="image_download_headers" for backwards compatibility, but BaseMarqoModel.Config never set allow_population_by_field_name. Pydantic v1 then only accepts the alias as input, and with extra = "forbid" a request that actually uses the documented camelCase field name (imageDownloadHeaders) is rejected outright as an unrecognized extra field. Enable allow_population_by_field_name on BaseMarqoModel, matching the pattern already used by base_model.py and recency_parameters.py, so both the documented imageDownloadHeaders and the deprecated image_download_headers alias are accepted. Fixes marqo-ai#401
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
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.
Summary
SearchQuery.imageDownloadHeaders(components/marqo/src/marqo/tensor_search/models/api_models.py) is declared withalias="image_download_headers", and the docstring/validator comments make clear the intent is:imageDownloadHeadersis the current field name,image_download_headersis kept only as a deprecated alias for backwards compatibility.But
BaseMarqoModel.Confignever setsallow_population_by_field_name. In pydantic v1, when a field has analias, only the alias is accepted as input unless that flag is set — and combined withextra = "forbid", a request actually using the documented camelCaseimageDownloadHeadersis rejected outright as an unrecognized field:So today only the deprecated snake_case name works, which is backwards from the documented behavior.
Fix
Set
allow_population_by_field_name = TrueonBaseMarqoModel.Config. This is the same pattern already used elsewhere in the codebase for exactly this kind of alias (base_model.py,recency_parameters.py), so it's consistent with existing convention.SearchQueryandBulkSearchQueryare the only classes built onBaseMarqoModel, andRecommendQuery(also built on it) declares no aliased fields, so this is scoped to the one place that needed it.Fixes #401
Test plan
test_image_download_headers_accepts_camel_case_field_name, assertingSearchQuery(imageDownloadHeaders=...)is accepted and correctly copied intomediaDownloadHeadersextra fields not permittederror when the fix is reverted, and passes with it appliedtests/unit_tests/marqo/tensor_search/models/test_api_models.py— 71 passed, 45 subtests passed (Python 3.11, perpyproject.toml'srequires-python)