-
Notifications
You must be signed in to change notification settings - Fork 62
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
dekaf: Generate predictable shard IDs for dekaf materialization endpoints #1848
Conversation
@@ -6,6 +6,16 @@ on: | |||
paths: | |||
- "crates/dekaf/**" | |||
- "crates/proto-**" | |||
- "crates/allocator" |
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.
I think I'm coming around to the idea that Dekaf should just be another binary in the flow image, as opposed to its own image. But for the moment, this just has it rebuild when any of its local crate deps change
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
@@ -327,7 +327,22 @@ async fn walk_materialization( | |||
{ | |||
shard_template.id.clone() | |||
} else { | |||
assemble::shard_id_prefix(pub_id, materialization, labels::TASK_TYPE_MATERIALIZATION) | |||
assemble::shard_id_prefix( | |||
match endpoint { |
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.
optional nit: feel free to disagree, but I found this inlined match expression to be a little hard on the eyes, and would I think prefer a variable.
…ints For UX reasons, we only want to use the task name (`myTenant/materializations/dekaf`) when authenticating to Dekaf, as opposed to the full shard template ID (`/myTenant/materializations/dekaf/0fa42cafc58003fa`). The agent API's `/authorize/task` expects to be passed the full shard template ID, so since Dekaf materializations never actually create any shards, we decided to use a fixed value for the pub ID component, allowing Dekaf to craft the correct shard template ID given only the task name.
3a188cc
to
fe4f57c
Compare
For UX reasons, we only want to use the task name (
myTenant/materializations/dekaf
) when authenticating to Dekaf, as opposed to the full shard template ID (/myTenant/materializations/dekaf/0fa42cafc58003fa
).OTOH, the agent API's
/authorize/task
endpoint expects to be passed the full shard template ID, so since Dekaf materializations never actually create any shards, we decided to use a fixed value for the pub ID component, allowing Dekaf to craft the correct shard template ID given only the task name.Cherry-picked out of #1840, as it's required to test Dekaf materialization functionality e2e
This change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)