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

Provide a Coroutine variant of ScanStream/ScanIterator #1438

Closed
wants to merge 1 commit into from

Conversation

sokomishalov
Copy link
Collaborator

@sokomishalov sokomishalov commented Oct 1, 2020

Implements #1435

P.S. Actually I'm not sure about the extension's signature due to the compiler conflict with existing member methods.
It makes end-users provide import aliases (see ScanStreamCoroutinesIntegrationTests).
Also maybe we should not underly on the ScanStream and provide separate and independent one.

@codecov
Copy link

codecov bot commented Oct 1, 2020

Codecov Report

Merging #1438 into main will decrease coverage by 0.03%.
The diff coverage is 82.05%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1438      +/-   ##
============================================
- Coverage     78.82%   78.79%   -0.04%     
- Complexity     6183     6219      +36     
============================================
  Files           460      462       +2     
  Lines         20800    20774      -26     
  Branches       2302     2295       -7     
============================================
- Hits          16396    16369      -27     
- Misses         3338     3345       +7     
+ Partials       1066     1060       -6     
Impacted Files Coverage Δ Complexity Δ
...pi/coroutines/RedisCoroutinesCommandsExtensions.kt 0.00% <0.00%> (ø) 0.00 <0.00> (?)
src/main/kotlin/io/lettuce/core/ScanFlow.kt 61.29% <61.29%> (ø) 10.00 <10.00> (?)
...i/coroutines/RedisClusterCoroutinesCommandsImpl.kt 81.25% <92.85%> (ø) 1.00 <1.00> (?)
...tuce/core/api/StatefulRedisConnectionExtensions.kt 100.00% <100.00%> (ø) 0.00 <0.00> (ø)
.../api/coroutines/BaseRedisCoroutinesCommandsImpl.kt 5.55% <100.00%> (ø) 1.00 <1.00> (?)
...core/api/coroutines/RedisCoroutinesCommandsImpl.kt 78.94% <100.00%> (ø) 2.00 <2.00> (?)
...e/api/coroutines/RedisGeoCoroutinesCommandsImpl.kt 8.33% <100.00%> (ø) 1.00 <1.00> (?)
...e/api/coroutines/RedisHLLCoroutinesCommandsImpl.kt 25.00% <100.00%> (ø) 1.00 <1.00> (?)
.../api/coroutines/RedisHashCoroutinesCommandsImpl.kt 10.00% <100.00%> (ø) 2.00 <1.00> (?)
...e/api/coroutines/RedisKeyCoroutinesCommandsImpl.kt 2.85% <100.00%> (ø) 1.00 <1.00> (?)
... and 41 more

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 535bf90...1220007. Read the comment docs.

@sokomishalov sokomishalov force-pushed the feaure/issue-1435 branch 2 times, most recently from d1ec2d7 to fe83f0b Compare October 2, 2020 09:57
@mp911de mp911de added this to the 6.1 M1 milestone Oct 2, 2020
@mp911de mp911de added the type: feature A new feature label Oct 2, 2020
@mp911de
Copy link
Collaborator

mp911de commented Oct 2, 2020

Moving this into the next iteration. Will review this PR after the release of 6.0.

Copy link
Collaborator

@mp911de mp911de left a comment

Choose a reason for hiding this comment

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

I added two comments, one about the overall design, one about splitting this PR into two commits to keep commits focussed on the actual topic.

@sokomishalov sokomishalov force-pushed the feaure/issue-1435 branch 2 times, most recently from bb4238a to 3b30ade Compare October 26, 2020 12:54
@sokomishalov sokomishalov marked this pull request as ready for review October 26, 2020 12:58
@sokomishalov sokomishalov requested a review from mp911de October 26, 2020 13:11
@mp911de mp911de self-assigned this Nov 2, 2020
@mp911de
Copy link
Collaborator

mp911de commented Nov 2, 2020

I'm going to take this PR from here.

mp911de pushed a commit that referenced this pull request Nov 2, 2020
We now provide ScanFlow as adapter for ScanStream.

Original pull request: #1438.
mp911de added a commit that referenced this pull request Nov 2, 2020
Update what's new section. Remove JvmOverloads annotation since ScanFlow isn't intended to be used from Java or other JVM languages. Add support and test for zscan. Reduce iterations to 300. Remove superfluous kotlin-stdlib-jdk8 dependency.

Add since tag and license header.

Original pull request: #1438.
@mp911de
Copy link
Collaborator

mp911de commented Nov 2, 2020

Thank you for your contribution. That's merged and polished now.

@mp911de mp911de closed this Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants