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

Support connector hints #117

Merged
merged 9 commits into from
Feb 27, 2025
Merged

Support connector hints #117

merged 9 commits into from
Feb 27, 2025

Conversation

jogrogan
Copy link
Collaborator

@jogrogan jogrogan commented Feb 26, 2025

Two changes:

  • Adds hints to source objects to show up in the connector. It's a very real use case where a user may want to pass through hint information maybe as to the consumer group or something else configurable in flink.
  • Passes through a "pipelineName" that will show up in the source/sink. This will be necessary for template parameters that require pipeline-uniqueness.

@ryannedolan
Copy link
Collaborator

re viewName, K8sMaterializedViewDeployer sets {{name}} for pipeline uniqueness purposes. Do we need more than that?

@jogrogan
Copy link
Collaborator Author

jogrogan commented Feb 26, 2025

re viewName, K8sMaterializedViewDeployer sets {{name}} for pipeline uniqueness purposes. Do we need more than that?

Name for the Source/Sink/Connector deployers doesn't include the partial view information:
Ex:

create or replace materialized view kafka."existing-topic-1$test" as select * from kafka."existing-topic-2";

yields {{name}} == "kafka-database-existing-topic-2"

If you have many partial views on the same topic, it won't be unique. Imagine the case of setting a specific consumer group for each source, it would need to be unique to this pipeline

@ryannedolan
Copy link
Collaborator

yields {{name}} == "kafka-database-existing-topic-2"

It should yield kakfa-database-existingtopic2-test. Maybe we broke that? I bet I broke it when I changed K8sMaterializedViewDeployer.

Copy link
Collaborator

@ryannedolan ryannedolan left a comment

Choose a reason for hiding this comment

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

I think partialViewName is a bit leaky, since the planner doesn't otherwise know about partial views. Could we get away with pipelineName here?

For the intended use-cases (e.g. consumer groups, client IDs, etc), can they just use the full pipeline name, e.g. kafka-database-existingtopic1-foo?

Copy link
Collaborator

@ryannedolan ryannedolan left a comment

Choose a reason for hiding this comment

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

Sorry for sending you around in circles. Make sense, thanks.

@jogrogan jogrogan merged commit 3f1a571 into main Feb 27, 2025
1 check passed
@jogrogan jogrogan deleted the jogrogan/connectorHints branch February 27, 2025 00:56
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