Skip to content

Minor change in poll method. #495

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

Merged
merged 1 commit into from
Dec 21, 2021
Merged

Conversation

OleksiiZubko
Copy link
Collaborator

@OleksiiZubko OleksiiZubko commented Dec 13, 2021

The issue is about sending POST requests for committing ofsets in poll method
even if no messages in poll response.
The backend sends back an error about invalid parameters of committing offsets
requests in this case.

This fix checks the count of messages in the poll
response and sends committing requests only if messages are retrieved.

Relates-To: OLPEDGE-2663

@codecov
Copy link

codecov bot commented Dec 13, 2021

Codecov Report

Merging #495 (bdb2469) into master (f3f336c) will increase coverage by 0.6133%.
The diff coverage is 90.0000%.

Impacted file tree graph

@@               Coverage Diff                @@
##             master       #495        +/-   ##
================================================
+ Coverage   88.9567%   89.5700%   +0.6133%     
================================================
  Files            82         82                
  Lines          2952       2953         +1     
  Branches        329        330         +1     
================================================
+ Hits           2626       2645        +19     
+ Misses          178        153        -25     
- Partials        148        155         +7     
Impacted Files Coverage Δ
...k-dataservice-read/lib/client/StreamLayerClient.ts 82.8572% <90.0000%> (+3.1471%) ⬆️
...ice-read/lib/cache/QuadTreeIndexCacheRepository.ts 25.0000% <0.0000%> (+12.5000%) ⬆️
...here/olp-sdk-dataservice-read/lib/utils/getTile.ts 40.7408% <0.0000%> (+27.7778%) ⬆️

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 f3f336c...bdb2469. Read the comment docs.

@OleksiiZubko OleksiiZubko force-pushed the OLPEDGE-2663-fix-stream-layer branch 4 times, most recently from 96e9cbc to 2a36532 Compare December 13, 2021 01:39
Copy link
Collaborator

@andescu andescu left a comment

Choose a reason for hiding this comment

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

The code looks good to me, but the commit message does not look good. Please explain the problem we want to solve and how do we solve it.

@OleksiiZubko OleksiiZubko force-pushed the OLPEDGE-2663-fix-stream-layer branch from 2a36532 to f752c63 Compare December 21, 2021 06:50
@OleksiiZubko OleksiiZubko requested a review from andescu December 21, 2021 06:51
andescu
andescu previously approved these changes Dec 21, 2021
The issue is about sending POST requests for committing ofsets
in `poll` method even if no messages in `poll` response.
The backend sends back an error about invalid parameters of
committing offsets requests in this case.

This fix checks the count of messages in the poll
response and sends committing requests only if messages are retrieved.

Relates-To: OLPEDGE-2663
Signed-off-by: Oleksii Zubko <[email protected]>
@OleksiiZubko OleksiiZubko merged commit 3b185fa into master Dec 21, 2021
@OleksiiZubko OleksiiZubko deleted the OLPEDGE-2663-fix-stream-layer branch December 21, 2021 07:49
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.

3 participants