Skip to content
This repository has been archived by the owner on Jan 19, 2022. It is now read-only.

Add maxMessages to PubSubReactiveFactory.poll #2441

Merged
merged 4 commits into from
Jun 26, 2020
Merged

Add maxMessages to PubSubReactiveFactory.poll #2441

merged 4 commits into from
Jun 26, 2020

Conversation

mturbe
Copy link
Contributor

@mturbe mturbe commented Jun 19, 2020

Fixes #2072

@codecov
Copy link

codecov bot commented Jun 19, 2020

Codecov Report

Merging #2441 into master will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2441      +/-   ##
============================================
+ Coverage     74.10%   74.14%   +0.03%     
- Complexity     2121     2123       +2     
============================================
  Files           264      264              
  Lines          7654     7657       +3     
  Branches        792      792              
============================================
+ Hits           5672     5677       +5     
+ Misses         1619     1617       -2     
  Partials        363      363              
Flag Coverage Δ Complexity Δ
#unittests 74.14% <100.00%> (+0.03%) 2123.00 <4.00> (+2.00)
Impacted Files Coverage Δ Complexity Δ
...oud/gcp/pubsub/reactive/PubSubReactiveFactory.java 88.09% <100.00%> (+0.91%) 16.00 <4.00> (+2.00)
...tastore/GcpDatastoreEmulatorAutoConfiguration.java 72.41% <0.00%> (+6.89%) 6.00% <0.00%> (ø%)

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 a3c9a04...d89a52e. Read the comment docs.

@dzou dzou added the pubsub GCP PubSub label Jun 19, 2020
@meltsufin meltsufin requested a review from elefeint June 23, 2020 16:38
Copy link
Contributor

@elefeint elefeint left a comment

Choose a reason for hiding this comment

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

Thank you for the help! Your PR is very well done.
I'd like @meltsufin 's take tomorrow on how to acquire max messages; depending on the outcome you may either be able to merge this version (with a small validation change) or update the logic to accept maxMessages as a constructor argument.

@mturbe
Copy link
Contributor Author

mturbe commented Jun 25, 2020

@elefeint, @dzou, @meltsufin: thank you for the feedback. I have reverted my previous commit and added a new overloaded constructor for PubSubReactiveFactory that has maxMessages as a parameter.

Copy link
Contributor

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

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

Thanks @mturbe!

Copy link
Contributor

@elefeint elefeint left a comment

Choose a reason for hiding this comment

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

Thank you!

@elefeint elefeint merged commit 02448ae into spring-attic:master Jun 26, 2020
@elefeint
Copy link
Contributor

As follow-up, I've filed #2451 to autoconfigure the new property.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pubsub GCP PubSub
Development

Successfully merging this pull request may close these issues.

PubSubReactiveFactory should allow overriding the pull batch size
4 participants