-
Notifications
You must be signed in to change notification settings - Fork 459
support creating index on update if missing #5773
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
fn update_index_qp() -> impl Filter<Extract = (UpdateQueryParams,), Error = Rejection> + Clone { | ||
serde_qs::warp::query::<UpdateQueryParams>(serde_qs::Config::default()) | ||
} |
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.
It's ok to inline this this given the function signature is longer than its content 😄
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.
yeah but calling that function is imo easier to read (and shorter) than the whole serde_qs line would be if it was copied to the callsite
i can inline if you prefer, but i personally don't
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.
No strong opinion. It is usually inlined in the rest of the codebase (e.g ingest_filter, search filters), but not always (extract_format_from_qs).
sandbox | ||
.rest_client(QuickwitService::Indexer) | ||
.indexes() | ||
.update( | ||
"my-updatable-index", | ||
r#" | ||
version: 0.8 | ||
index_id: my-updatable-index | ||
doc_mapping: | ||
field_mappings: | ||
- name: title | ||
type: text | ||
- name: body | ||
type: text | ||
indexing_settings: | ||
commit_timeout_secs: 1 | ||
search_settings: | ||
default_search_fields: [title, body] | ||
"#, | ||
quickwit_config::ConfigFormat::Yaml, | ||
true, | ||
) | ||
.await | ||
.unwrap(); |
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.
nit, I think PUT is supposed to return 201 if the ressource was created
@trinity-1686a Could you also update the docs? |
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.
LGTM! Thanks for adding the source upsert as well 🙂
Description
when sending an index update, optionally create the index if it didn't exist previously
for coherency, we could support that feature for sources too, this isn't implemented
How was this PR tested?
manually tested, added tests