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.

Sign up for GitHub

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

Jump to bottom

Issue #3937: Paged flux draft PR #4045

Merged
merged 16 commits into from
Jun 28, 2019
Merged

Issue #3937: Paged flux draft PR #4045

merged 16 commits into from
Jun 28, 2019

Conversation

srnagar
Copy link
Member

@srnagar srnagar commented Jun 21, 2019

Draft prototype of paged flux to get some early feedback on the general direction of this implementation.

I have also updated ConfigurationAsyncClient implementation for listSettings to return a paged flux to verify that all existing tests are passing using the item flux. After addressing all review feedback, I will add tests to ConfigurationAsyncClient to test subscription by page.

Logs from PagedFluxTest:

-------------- Running testPagedFluxSubscribeToItems -----------------------------
[main] INFO reactor.Flux.Paged.1 - onSubscribe(FluxConcatArray.ConcatArraySubscriber)
[main] INFO reactor.Flux.Paged.1 - request(unbounded)
[main] INFO reactor.Flux.Paged.1 - onNext(0)
[main] INFO reactor.Flux.Paged.1 - onNext(1)
[main] INFO reactor.Flux.Paged.1 - onNext(2)
[main] INFO reactor.Flux.Paged.1 - onNext(3)
[main] INFO reactor.Flux.Paged.1 - onNext(4)
[main] INFO reactor.Flux.Paged.1 - onNext(5)
[main] INFO reactor.Flux.Paged.1 - onNext(6)
[main] INFO reactor.Flux.Paged.1 - onNext(7)
[main] INFO reactor.Flux.Paged.1 - onNext(8)
[main] INFO reactor.Flux.Paged.1 - onNext(9)
[main] INFO reactor.Flux.Paged.1 - onNext(10)
[main] INFO reactor.Flux.Paged.1 - onNext(11)
[main] INFO reactor.Flux.Paged.1 - onNext(12)
[main] INFO reactor.Flux.Paged.1 - onNext(13)
[main] INFO reactor.Flux.Paged.1 - onNext(14)
[main] INFO reactor.Flux.Paged.1 - onComplete()
-------------- Running testPagedFluxSubscribeToPages -----------------------------
[main] INFO reactor.Flux.MonoFlatMapMany.2 - onSubscribe(FluxConcatArray.ConcatArraySubscriber)
[main] INFO reactor.Flux.MonoFlatMapMany.2 - request(unbounded)
[main] INFO reactor.Flux.MonoFlatMapMany.2 - onNext(com.azure.core.implementation.http.PagedResponseBase@9d5509a)
[main] INFO reactor.Flux.MonoFlatMapMany.2 - onNext(com.azure.core.implementation.http.PagedResponseBase@179ece50)
[main] INFO reactor.Flux.MonoFlatMapMany.2 - onNext(com.azure.core.implementation.http.PagedResponseBase@3b0090a4)
[main] INFO reactor.Flux.MonoFlatMapMany.2 - onNext(com.azure.core.implementation.http.PagedResponseBase@3cd3e762)
[main] INFO reactor.Flux.MonoFlatMapMany.2 - onNext(com.azure.core.implementation.http.PagedResponseBase@1fa121e2)
[main] INFO reactor.Flux.MonoFlatMapMany.2 - onComplete()
-------------- Running testEmptyFirstPage -----------------------------
[main] INFO reactor.Flux.Paged.3 - onSubscribe([Fuseable] Operators.EmptySubscription)
[main] INFO reactor.Flux.Paged.3 - request(unbounded)
[main] INFO reactor.Flux.Paged.3 - onComplete()
-------------- Running testPagedFluxSubscribeToPageWithContinuationToken -----------------------------
[main] INFO reactor.Flux.MonoFlatMapMany.4 - onSubscribe(FluxConcatArray.ConcatArraySubscriber)
[main] INFO reactor.Flux.MonoFlatMapMany.4 - request(unbounded)
[main] INFO reactor.Flux.MonoFlatMapMany.4 - onNext(com.azure.core.implementation.http.PagedResponseBase@ba2f4ec)
[main] INFO reactor.Flux.MonoFlatMapMany.4 - onNext(com.azure.core.implementation.http.PagedResponseBase@1c1bbc4e)
[main] INFO reactor.Flux.MonoFlatMapMany.4 - onComplete()

@srnagar srnagar requested a review from JonathanGiles June 21, 2019 20:02
JonathanGiles
JonathanGiles requested changes Jun 26, 2019
View reviewed changes
Copy link
Member

@JonathanGiles JonathanGiles left a comment

Choose a reason for hiding this comment

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

Generally looks good. Focus on tests and docs now.

@srnagar srnagar requested a review from JonathanGiles June 27, 2019 07:49
@srnagar srnagar marked this pull request as ready for review June 27, 2019 17:42
@srnagar srnagar requested a review from JonathanGiles June 27, 2019 22:32
JonathanGiles
JonathanGiles reviewed Jun 27, 2019
View reviewed changes
core/azure-core/pom.xml Outdated
<scope>provided</scope>
</dependency>


Copy link
Member

Choose a reason for hiding this comment

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

One too many newlines.

@srnagar srnagar merged commit bd592d3 into Azure:master Jun 28, 2019
@srnagar srnagar mentioned this pull request Jun 28, 2019
Closed
sima-zhu pushed a commit that referenced this pull request Jun 29, 2019
@srnagar @sima-zhu
Issue #3937: Paged flux draft PR (#4045)
3331258 
* Initial prototype for paged flux

* Update unit tests and javadocs

* Remove unused imports

* Updated to use paged response instead of continuation token

* Update javadocs

* Supplier and function

* Update javadocs, add code snippets and more unit tests

* Undo package-info changes

* Undo package-info changes

* Update javadocs

* Add jsr dependency

* Remove extra blank line
sima-zhu pushed a commit to sima-zhu/azure-sdk-for-java that referenced this pull request Jun 29, 2019
@srnagar @sima-zhu
Issue Azure#3937: Paged flux draft PR (Azure#4045)
7e328f1 
* Initial prototype for paged flux

* Update unit tests and javadocs

* Remove unused imports

* Updated to use paged response instead of continuation token

* Update javadocs

* Supplier and function

* Update javadocs, add code snippets and more unit tests

* Undo package-info changes

* Undo package-info changes

* Update javadocs

* Add jsr dependency

* Remove extra blank line
@srnagar srnagar deleted the paged-flux branch July 9, 2019 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alzimmermsft alzimmermsft alzimmermsft approved these changes

@JonathanGiles JonathanGiles JonathanGiles approved these changes

@conniey conniey Awaiting requested review from conniey

@jianghaolu jianghaolu Awaiting requested review from jianghaolu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@srnagar @JonathanGiles @alzimmermsft