Skip to content

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

Merged
merged 8 commits into from
Jun 6, 2025
Merged

Conversation

trinity-1686a
Copy link
Contributor

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

@trinity-1686a trinity-1686a requested a review from guilload May 21, 2025 09:28
Comment on lines +296 to +298
fn update_index_qp() -> impl Filter<Extract = (UpdateQueryParams,), Error = Rejection> + Clone {
serde_qs::warp::query::<UpdateQueryParams>(serde_qs::Config::default())
}
Copy link
Collaborator

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 😄

Copy link
Contributor Author

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

Copy link
Collaborator

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).

Comment on lines +124 to +147
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();
Copy link
Collaborator

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

@rdettai
Copy link
Collaborator

rdettai commented May 21, 2025

@trinity-1686a Could you also update the docs?

@trinity-1686a trinity-1686a requested a review from rdettai May 21, 2025 14:47
Copy link
Collaborator

@rdettai rdettai left a 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 🙂

@trinity-1686a trinity-1686a merged commit dc54c8e into main Jun 6, 2025
8 checks passed
@trinity-1686a trinity-1686a deleted the 1686a/update-create branch June 6, 2025 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants