-
Notifications
You must be signed in to change notification settings - Fork 415
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
Add update source API #5636
base: main
Are you sure you want to change the base?
Add update source API #5636
Conversation
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.
In good shape so far. I have yet to review the quickwit-serve
part.
quickwit/quickwit-metastore/src/metastore/index_metadata/mod.rs
Outdated
Show resolved
Hide resolved
quickwit/quickwit-metastore/src/metastore/postgres/metastore.rs
Outdated
Show resolved
Hide resolved
6f7bcd2
to
39a8506
Compare
39a8506
to
abf7723
Compare
cc21afd
to
d7c318d
Compare
Currently the pipeline fingerpring only contains index settings. The source setting changes should also trigger a pipeline restart. Note that this change will change all the existing pipeline fingerprints. This means that once the control plane runs this for the first time, it will restart all indexing pipelines.
Also include some comment and naming improvments.
b630b98
to
ce61a97
Compare
let mut hasher = SipHasher::new(); | ||
self.doc_mapping.doc_mapping_uid.hash(&mut hasher); | ||
self.indexing_settings.hash(&mut hasher); | ||
hasher.finish() | ||
} | ||
|
||
/// Compare IndexConfig level fingerprints |
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.
/// Compare IndexConfig level fingerprints | |
/// Compares IndexConfig level fingerprints |
fn validate_update(&self, _other: &Self) -> anyhow::Result<()> { | ||
// In the Pulsar source, we use use combinations of the topic+partition | ||
// (generated by the Pulsar client library) as metastore checkpoint | ||
// PartitionId, and those are guarantied to be unique across topics. |
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.
// PartitionId, and those are guarantied to be unique across topics. | |
// PartitionId, and those are "guaranteed" to be unique across topics. |
@@ -47,6 +47,7 @@ tracing = { workspace = true } | |||
ulid = { workspace = true } | |||
utoipa = { workspace = true } | |||
vrl = { workspace = true, optional = true } | |||
warp = { workspace = true, optional = true } |
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.
Can warp
be a dev-dependency?
Description
Closes #5634
How was this PR tested?
Describe how you tested this PR.