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

Improve ergonomics of creating GrpcReadJournal, #703 #705

Merged
merged 3 commits into from
Oct 12, 2022

Conversation

patriknw
Copy link
Member

  • withAdditionalRequestMetadata for optional metadata
  • Convenience factory methods for GrpcReadJournal

Draft because on top of other PR, but otherwise ready for review.

Refs #703

@patriknw patriknw marked this pull request as draft October 11, 2022 09:05
Base automatically changed from wip-fixme-patriknw to main October 12, 2022 08:47
Copy link
Member

@johanandren johanandren left a comment

Choose a reason for hiding this comment

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

LGTM, one suggestion

@@ -108,7 +105,7 @@ public static void init(ActorSystem<?> system) {
int numberOfProjectionInstances = 4;
String projectionName = "cart-events";
List<Pair<Integer, Integer>> sliceRanges = Persistence.get(system).getSliceRanges(numberOfProjectionInstances);
String streamId = "cart";
String streamId = system.settings().config().getString("akka.projection.grpc.consumer.stream-id");
Copy link
Member

Choose a reason for hiding this comment

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

Should we make the streamId a public field on GrpcReadJournal, so that even if it needs to be passed to the source provider at least this isn't needed and you know it's the same used for the journal?

Copy link
Member Author

Choose a reason for hiding this comment

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

Det låter som en bra ide. Så parametern till SourceProvider blir eventsBySlicesQuery.streamId.

Copy link
Member

Choose a reason for hiding this comment

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

lol 🇸🇪

Copy link
Member Author

Choose a reason for hiding this comment

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

😄 added in d581fc1

@patriknw patriknw marked this pull request as ready for review October 12, 2022 09:51
@patriknw patriknw merged commit b5dc0e5 into main Oct 12, 2022
@patriknw patriknw deleted the wip-703-ergo-patriknw branch October 12, 2022 12:14
@patriknw patriknw modified the milestones: 1.3.0-M3, 1.3.0-M4 Oct 12, 2022
@patriknw patriknw modified the milestones: 1.3.0-M4, 1.3.0 Oct 20, 2022
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.

None yet

2 participants