Skip to content

chore: adapt to the new API for NamedTopology builders and runtimes#8331

Merged
A. Sophie Blee-Goldman (ableegoldman) merged 3 commits into
confluentinc:masterfrom
ableegoldman:basic-updates-to-work-with-new-NamedTopology-API
Nov 18, 2021
Merged

chore: adapt to the new API for NamedTopology builders and runtimes#8331
A. Sophie Blee-Goldman (ableegoldman) merged 3 commits into
confluentinc:masterfrom
ableegoldman:basic-updates-to-work-with-new-NamedTopology-API

Conversation

@ableegoldman

Copy link
Copy Markdown
Contributor

Extracting some self-contained pieces of the mega-PR #8219 before I tackle any of the large refactoring that was suggested.

This PR just prepares only the necessary changes for us to finally merge https://github.com/apache/kafka/pull/11272 without breaking the build. Since we were already starting up an empty KafkaStreams, the only changes we need here are to get the TopologyBuilder from this KafkaStreams

return Objects.equals(sources, queryPlan.sources)
&& Objects.equals(sink, queryPlan.sink)
&& Objects.equals(physicalPlan, queryPlan.physicalPlan)
&& Objects.equals(runtimeId, queryPlan.runtimeId)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

just a small fix on the side that I kept in here

Comment on lines +66 to +69
private final Map<QueryId, PersistentQueryMetadata> persistentQueries = new ConcurrentHashMap<>();
private final Map<QueryId, QueryMetadata> allLiveQueries = new ConcurrentHashMap<>();
private final Map<SourceName, QueryId> createAsQueries = new ConcurrentHashMap<>();
private final Map<SourceName, Set<QueryId>> insertQueries = new ConcurrentHashMap<>();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

again, just a small refactoring that I figured we may as well pull into this PR since it's so small

@ableegoldman

Copy link
Copy Markdown
Contributor Author

Walker Carlson (@wcarlson5) Rohan (@rodesai) I'm working on breaking up that one large PR into smaller chunks that we can start to merge right away, and unblock some other work, please take a look

@wcarlson5 Walker Carlson (wcarlson5) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM once the build passes

@wcarlson5

Walker Carlson (wcarlson5) commented Nov 10, 2021

Copy link
Copy Markdown
Contributor

looks like you have a compilation error

09:50:43  [INFO] -------------------------------------------------------------
09:50:43  [ERROR] /home/jenkins/workspace/entinc_Contributors_ksql_PR-8331/ksqldb-engine/src/main/java/io/confluent/ksql/query/QueryBuilder.java:[95,66] cannot find symbol

@ableegoldman

Copy link
Copy Markdown
Contributor Author

Walker Carlson (@wcarlson5) this isn't going to compile/pass the PR build until this gets synced over to ccs-kafka, ,and the sync job is currently blocked on conflicts due to the 3.2 version bump.

I ran the tests locally with the above commit and verified they pass so we should be good to merge this right away after the sync job goes through

@ableegoldman A. Sophie Blee-Goldman (ableegoldman) force-pushed the basic-updates-to-work-with-new-NamedTopology-API branch from 9b00089 to 84094e7 Compare November 11, 2021 05:52
@ableegoldman A. Sophie Blee-Goldman (ableegoldman) force-pushed the basic-updates-to-work-with-new-NamedTopology-API branch from 84094e7 to 791c375 Compare November 18, 2021 18:09
@ableegoldman A. Sophie Blee-Goldman (ableegoldman) force-pushed the basic-updates-to-work-with-new-NamedTopology-API branch from 791c375 to c05a08a Compare November 18, 2021 18:15
@ableegoldman A. Sophie Blee-Goldman (ableegoldman) merged commit c938175 into confluentinc:master Nov 18, 2021
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