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

Decoupled topology #278

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

markbastian
Copy link

Checklist

  • tests
    • Note that there were 3 failing tests on master. These are still failing here.
  • updated CHANGELOG (the "unreleased" section)

* Renamed test ns to correct file path
* Modified jackdaw.streams/kafka-streams to accept either a Topology or a CljStreamsBuilder.
@markbastian markbastian requested a review from a team as a code owner November 14, 2020 23:14
@@ -16,3 +16,5 @@ pom.xml.asc
.lein-*
.cpcache
.nrepl-port
/.idea
Copy link
Author

Choose a reason for hiding this comment

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

This is a convenience for IntelliJ/Cursive users that prevents project files from being committed.

(.putAll props opts)
(KafkaStreams. ^Topology (.build ^StreamsBuilder (streams-builder* builder))
^java.util.Properties props))))
(defn ^Topology topology
Copy link
Author

Choose a reason for hiding this comment

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

This block decouples the creation of the streams app from the creation of the underlying Topology. This is particularly useful for drilling down into the Topology for doing things like accessing the description as well as opening up the way to doing datafy/nav using the active system's topology.

@@ -1,4 +1,4 @@
(ns jackdaw.client-test
(ns jackdaw.client.partitioning-test
Copy link
Author

Choose a reason for hiding this comment

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

This is the correct ns name (unless you want to move the file).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah thanks for fixing this.

@codecov
Copy link

codecov bot commented Nov 14, 2020

Codecov Report

Merging #278 (8f8b24e) into master (daa8387) will increase coverage by 0.17%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #278      +/-   ##
==========================================
+ Coverage   81.16%   81.34%   +0.17%     
==========================================
  Files          41       41              
  Lines        2586     2589       +3     
  Branches      153      153              
==========================================
+ Hits         2099     2106       +7     
+ Misses        334      330       -4     
  Partials      153      153              
Impacted Files Coverage Δ
src/jackdaw/streams.clj 83.59% <100.00%> (+0.39%) ⬆️
src/jackdaw/serdes/edn2.clj 45.45% <0.00%> (-45.46%) ⬇️
src/jackdaw/client/partitioning.clj 64.10% <0.00%> (+35.89%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update daa8387...8f8b24e. Read the comment docs.

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