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

Introduce MaterializationEndpoint::Dekaf #1642

Closed
wants to merge 4 commits into from

Conversation

jshearer
Copy link
Contributor

@jshearer jshearer commented Sep 19, 2024

Description:

This is the first part of #1622. It introduces a new materialization endpoint type called dekaf, as well as the necessary control-plane plumbing to give materializations of this type ops logs and stats journals when activated. As materializations of this type are purely descriptive and are actually served by the stand-alone Dekaf service, no shards or recovery log journals should be created.

I added a new happy-path test to snapshot a new dekaf-type materialization, and am able to publish the following spec to my local stack, see it show up in the UI, and delete it successfully.

materializations: 
  aliceCo/dekaf-test:
    bindings:
      - source: aliceCo/test-hello/events
        resource: {}
        fields:
          recommended: true
    endpoint:
      dekaf: {}

This change is Reviewable

@jshearer jshearer added the change:planned This is a planned change label Sep 19, 2024
@jshearer jshearer requested review from psFried and removed request for psFried September 19, 2024 17:24
@jshearer jshearer changed the title Jshearer/dekaf materialization endpoint Introduce MaterializationEndpoint::Dekaf Sep 19, 2024
@jshearer jshearer force-pushed the jshearer/dekaf_materialization_endpoint branch 2 times, most recently from 96653f0 to 000a3a0 Compare September 19, 2024 20:08
@jshearer jshearer marked this pull request as ready for review September 20, 2024 18:27
@jshearer jshearer force-pushed the jshearer/dekaf_materialization_endpoint branch from cf898b0 to 6fa871d Compare September 20, 2024 18:43
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.

Looks good!

crates/activate/src/lib.rs Show resolved Hide resolved
crates/models/src/materializations.rs Outdated Show resolved Hide resolved
Copy link
Member

@jgraettinger jgraettinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % nits around config handling

crates/flowctl/src/generate/mod.rs Outdated Show resolved Hide resolved
crates/models/src/materializations.rs Outdated Show resolved Hide resolved
crates/sources/src/indirect.rs Outdated Show resolved Hide resolved
@jshearer jshearer force-pushed the jshearer/dekaf_materialization_endpoint branch from 78cae66 to 5a1f419 Compare September 24, 2024 21:05
@@ -41,6 +41,13 @@ pub struct MaterializationDef {
pub delete: bool,
}

#[derive(Serialize, Deserialize, Clone, Debug, JsonSchema, PartialEq)]
#[serde(untagged)]
pub enum DekafConfigContainer {
Copy link
Contributor Author

@jshearer jshearer Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jgraettinger This seem right to you? The other endpoint types use a RawValue for their config because they're dynamic, and that lets them inherently accept either a string (which is treated as an indirect reference), or an object (which is treated as an inline config), whereas the dekaf config is defined in-tree.

I contemplated whether I should generalize this Direct / Indirect config container to the other endpoint types to make that distinction more explicit, but it didn't seem warranted, more of a preference than tech debt. Thoughts?

@jshearer jshearer force-pushed the jshearer/dekaf_materialization_endpoint branch 2 times, most recently from b028eb4 to 47423d0 Compare September 25, 2024 14:54
A materialization of this kind is purely descriptive, and cannot be directly started. It is used to convey the intent to expose a set of bindings through Dekaf, which will look it up to determine things like binding names and field selection/projections.
…very journals, but they _should_ get ops stats+logs collections
@jshearer
Copy link
Contributor Author

jshearer commented Sep 30, 2024

Closing in favor of #1665 which contains some substantial changes from this PR which I didn't want to push here and confuse things

@jshearer jshearer closed this Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:planned This is a planned change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants