Skip to content
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

Merged
merged 2 commits into from
Jan 2, 2025

Conversation

jshearer
Copy link
Contributor

@jshearer jshearer commented Dec 30, 2024

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

@@ -6,6 +6,16 @@ on:
paths:
- "crates/dekaf/**"
- "crates/proto-**"
- "crates/allocator"
Copy link
Contributor Author

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

Copy link
Member

@psFried psFried left a 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 {
Copy link
Member

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