Paged Flux abstractions #6465

anuchandy · 2019-11-21T09:42:27Z

No description provided.

…col e.g. http)

sdk/core/azure-core/src/main/java/com/azure/core/paging/PagedFluxCore.java

…ing-base

…g code-snippets

… the default implementation of paged flux

…as PagedFlux, this will help us from not exposing more-more flux operator on PagedFlux, enable Continuation-option in base abstract type

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/PagedFlux.java

anuchandy · 2019-12-11T23:06:06Z

@JonathanGiles we talked about identifying the places where mapPage is used: It is used only by Data Lake SDK at the moment

azure-sdk-for-java/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeServiceAsyncClient.java

Line 259 in d6a8381

.mapPage(Transforms::toFileSystemItem);

sdk/core/azure-core/src/main/java/com/azure/core/util/paging/ContinuablePagedFluxCore.java

sdk/core/azure-core/src/main/java/com/azure/core/util/paging/ContinuationState.java

alzimmermsft · 2019-12-11T23:31:06Z

Would this handle only requesting 1 page at a time and not trigger subsequent page requests? #6518

…ontinuationState.java Co-Authored-By: Alan Zimmer <48699787+alzimmermsft@users.noreply.github.com>

anuchandy · 2019-12-12T00:01:39Z

@alzimmermsft Would this handle only requesting 1 page at a time and not trigger subsequent page requests: its configurable in the ContinuablePagedFluxCore Ctr through prefetch argument. If not specified we default to the reactor prefetch.

… into paging-base

…edFlux.java Co-Authored-By: Alan Zimmer <48699787+alzimmermsft@users.noreply.github.com>

sdk/core/azure-core/src/main/java/com/azure/core/paging/ContinuablePagedFlux.java

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/PagedFlux.java

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/PagedFluxBase.java

sdk/core/azure-core/src/main/java/com/azure/core/util/paging/ContinuablePage.java

sdk/core/azure-core/src/main/java/com/azure/core/util/paging/ContinuablePagedFlux.java

…ersion of azure-core to 1.2.0 (to get new page abstractions)

…torage-batch, azure-core-test to unreleased 1.2.0

srnagar

I have a few comments about docs and minor changes. Otherwise, looks great!

srnagar · 2020-01-14T03:54:17Z

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/Page.java

-     */
-    List<T> getItems();
-
+public interface Page<T> extends ContinuablePage<String, T> {


Yes, because we have a released version already, we'll have to keep this hierarchy.

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/Page.java

srnagar · 2020-01-14T03:57:49Z

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/PagedFlux.java

- * This class is a flux that can operate on a {@link PagedResponse} and also provides the ability to operate on
- * individual items. When processing the response by page, each response will contain the items in the page as well as
- * the request details like status code and headers.
+ * This type is a Flux provides the ability to operate on paginated REST response of type {@link PagedResponse}


Suggested change

* This type is a Flux provides the ability to operate on paginated REST response of type {@link PagedResponse}

* This type is a {@link Flux} that provides the ability to operate on paginated REST response of type {@link PagedResponse}

srnagar · 2020-01-14T04:00:32Z

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/PagedFlux.java

+    // 2. Add a new ctr        "PagedFlux(Supplier<PageRetriever<String, PagedResponse<T>>>)".
+    // 3. Remove the factory method "PagedFlux::create" and this PRIVATE ctr in favour of #2.
+    //
+    private PagedFlux(Supplier<PageRetriever<String, PagedResponse<T>>> provider, boolean ignored) {


Agree. Maybe this can be tracked in a github issue with appropriate labels so we don't forget to make this change when working on a major version release.

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/PagedFlux.java

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/PagedFluxBase.java

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/PagedResponseBase.java

sdk/core/azure-core/src/main/java/com/azure/core/util/paging/ContinuablePagedFluxCore.java

JonathanGiles · 2020-01-14T03:14:32Z

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/Page.java

-     */
-    List<T> getItems();
-
+public interface Page<T> extends ContinuablePage<String, T> {


@anuchandy Can you please be sure to update #6610 with the final set of changes we would make when a breaking-change release is possible.

JonathanGiles · 2020-01-14T03:16:45Z

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/Page.java

+    default List<T> getItems() {
+        IterableStream<T> iterableStream = this.getElements();
+        return iterableStream == null
+            ? null


Could we not return an empty IterableStream rather than null here, to be doubly protective of the end user?

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/PagedFlux.java

JonathanGiles · 2020-01-14T03:26:09Z

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/PagedFlux.java

+    // 2. Add a new ctr        "PagedFlux(Supplier<PageRetriever<String, PagedResponse<T>>>)".
+    // 3. Remove the factory method "PagedFlux::create" and this PRIVATE ctr in favour of #2.
+    //
+    private PagedFlux(Supplier<PageRetriever<String, PagedResponse<T>>> provider, boolean ignored) {


We will unfortunately need to live with this inconsistency until we can do a breaking change release. The only positive is that we don't expect end users to be dealing with this API, so the only people encountering this will be client library developers.

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/PagedFluxBase.java

JonathanGiles · 2020-01-14T23:42:21Z

sdk/core/azure-core/src/main/java/com/azure/core/implementation/serializer/ItemPage.java

-    public List<T> getItems() {
-        return items;
+    public IterableStream<T> getElements() {
+        return items == null


I agree with Alan

…ing-base

Co-Authored-By: Jonathan Giles <jonathan@jonathangiles.net>

JonathanGiles · 2020-01-17T01:46:53Z

...n/src/main/java/com/azure/data/appconfiguration/implementation/ConfigurationSettingPage.java

-    public List<ConfigurationSetting> getItems() {
-        return items;
+    public IterableStream<ConfigurationSetting> getElements() {
+        return items == null ? IterableStream.empty() : new IterableStream<>(items);


Could introduce IterableStream.of(List) or similar to avoid null checks in these cases.

added IterableStream.of(Iterable<T>) factory method: if parameter to this factory is null then it return cached static empty stream otherwise it create a new IterableStream from the parameter.

… into paging-base

JonathanGiles

LGTM. Let's get this into azure-core and have a beta release in February to test it out. @hemanttanwar We don't want the february release to be a GA release of azure core.

anuchandy · 2020-01-17T03:28:30Z

@hemanttanwar - Couple of client-lib pom are updated to refer "unreleased version of azure_core" which has pagination improvements. Could you please take a look at those and if all good let's get this in.

hemanttanwar

Like we spoke about it, Just run update_version script to make sure there are no more changed needed in pom files.

anuchandy · 2020-01-17T22:24:05Z

@hemanttanwar thanks, as discussed I ran the following command on this changes:

python eng/versioning/update_versions.py --ut library --bt client --sr

and it didn't update any pom files which indicates that the current changes are correct.

PagedFluxCore as an abstraction for generic paging (not tied to proto…

424490c

…col e.g. http)

anuchandy requested review from JonathanGiles, srnagar, alzimmermsft, hemanttanwar and jianghaolu November 21, 2019 10:03

Making PagedFluxCore abstract, letting user to define state type

35f8814

JonathanGiles reviewed Nov 21, 2019

View reviewed changes

sdk/core/azure-core/src/main/java/com/azure/core/paging/PagedFluxCore.java Outdated Show resolved Hide resolved

Making the Paged Flux type more flexiable

1eb7381

anuchandy changed the title ~~PagedFluxCore as an abstraction for generic paging (not tied to protocol e.g. http)~~ Paged Flux abstractions Nov 24, 2019

anuchandy added 2 commits November 25, 2019 10:53

Adding byItem(token) and some cleanup.

d3a5d42

Code cleanup and adding javadoc code-snippets for PagedFluxCore

1dee7f5

anuchandy marked this pull request as ready for review November 26, 2019 18:44

anuchandy added 2 commits November 26, 2019 10:54

Merge branch 'master' of github.com:azure/azure-sdk-for-java into pag…

7fd7e41

…ing-base

Moving paging base types to util package, adding exclusions for pagin…

92029bd

…g code-snippets

anuchandy requested a review from mssfang as a code owner November 26, 2019 19:50

anuchandy added 3 commits December 1, 2019 12:57

Making the base Paged Flux completely abstract, exposing prefetch for…

946f8ae

… the default implementation of paged flux

Enabling a way to decorate a PagedFlux and have that decoreated type …

8c69889

…as PagedFlux, this will help us from not exposing more-more flux operator on PagedFlux, enable Continuation-option in base abstract type

Removed unused internal ctr

1540c6c

alzimmermsft reviewed Dec 11, 2019

View reviewed changes

sdk/core/azure-core/src/main/java/com/azure/core/http/rest/PagedFlux.java Outdated Show resolved Hide resolved

conniey reviewed Dec 11, 2019

View reviewed changes

sdk/core/azure-core/src/main/java/com/azure/core/util/paging/ContinuablePagedFluxCore.java Outdated Show resolved Hide resolved

sdk/core/azure-core/src/main/java/com/azure/core/util/paging/ContinuablePagedFluxCore.java Outdated Show resolved Hide resolved

alzimmermsft reviewed Dec 11, 2019

View reviewed changes

sdk/core/azure-core/src/main/java/com/azure/core/util/paging/ContinuationState.java Outdated Show resolved Hide resolved

Update sdk/core/azure-core/src/main/java/com/azure/core/util/paging/C…

dfa1f40

…ontinuationState.java Co-Authored-By: Alan Zimmer <48699787+alzimmermsft@users.noreply.github.com>

anuchandy and others added 3 commits December 11, 2019 19:02

cleaning up comments and simplifying the code

b9fe5e9

Merge branch 'paging-base' of github.com:anuchandy/azure-sdk-for-java…

317b7d8

… into paging-base

Update sdk/core/azure-core/src/main/java/com/azure/core/http/rest/Pag…

6b82642

…edFlux.java Co-Authored-By: Alan Zimmer <48699787+alzimmermsft@users.noreply.github.com>

JonathanGiles requested changes Dec 12, 2019

View reviewed changes

anuchandy added 2 commits January 5, 2020 14:02

Udating keyvault, storage-blob-batch and azure-core-test referenced v…

879f7ca

…ersion of azure-core to 1.2.0 (to get new page abstractions)

Fixing checkstyles in textanalytics samples

ee078a8

anuchandy requested a review from joshfree as a code owner January 5, 2020 22:02

Explicitly specify type in code-snippet

5363af6

anuchandy force-pushed the paging-base branch from 0c7dea1 to 5363af6 Compare January 6, 2020 04:33

anuchandy added 2 commits January 5, 2020 23:38

updating azure-core version of kevault-certificates, keys, secrets, s…

f23ac6e

…torage-batch, azure-core-test to unreleased 1.2.0

Adding revapi supression for new base ContinuablePagedFluxCore

d78a70b

anuchandy force-pushed the paging-base branch from bf750a2 to d78a70b Compare January 6, 2020 20:02

JonathanGiles requested a review from srnagar January 14, 2020 03:11

srnagar approved these changes Jan 14, 2020

View reviewed changes

sync with upstream

8c5372f

JonathanGiles reviewed Jan 14, 2020

View reviewed changes

anuchandy and others added 6 commits January 14, 2020 16:04

bump up azure-core versions of client-libs that uses new page types

79b1543

Merge branch 'master' of github.com:azure/azure-sdk-for-java into pag…

d2e764f

…ing-base

Merge branch 'master' of github.com:azure/azure-sdk-for-java into pag…

dfe68d4

…ing-base

javadoc improvments

f01f1c9

Page::getElements() should not return null

ac3a8f4

PagedFlux javadoc update

b08ec16

Co-Authored-By: Jonathan Giles <jonathan@jonathangiles.net>

JonathanGiles reviewed Jan 17, 2020

View reviewed changes

anuchandy added 2 commits January 16, 2020 18:36

Adding IterableStream::of(Iterable) factory method

43e83e4

Merge branch 'paging-base' of github.com:anuchandy/azure-sdk-for-java…

44fcc6d

… into paging-base

JonathanGiles approved these changes Jan 17, 2020

View reviewed changes

anuchandy requested review from hemanttanwar and removed request for hemanttanwar January 17, 2020 03:28

hemanttanwar approved these changes Jan 17, 2020

View reviewed changes

anuchandy merged commit 570f57f into Azure:master Jan 17, 2020

alzimmermsft mentioned this pull request Jan 28, 2020

[FEATURE REQ] ADLS: Missing API - PagedIterable onErrorMap function to return a PagedIterable #6751

Closed

2 tasks

alzimmermsft mentioned this pull request Apr 20, 2020

[BUG] PagedIterable.iterableByPage(token) processes listing operations eagerly #6518

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Paged Flux abstractions #6465

Paged Flux abstractions #6465

anuchandy commented Nov 21, 2019

anuchandy commented Dec 11, 2019 •

edited

Loading

alzimmermsft commented Dec 11, 2019

anuchandy commented Dec 12, 2019

srnagar left a comment

srnagar Jan 14, 2020

srnagar Jan 14, 2020

srnagar Jan 14, 2020

JonathanGiles Jan 14, 2020

JonathanGiles Jan 14, 2020

JonathanGiles Jan 14, 2020

JonathanGiles Jan 14, 2020

JonathanGiles Jan 17, 2020

anuchandy Jan 17, 2020

JonathanGiles left a comment

anuchandy commented Jan 17, 2020

hemanttanwar left a comment

anuchandy commented Jan 17, 2020

	* This type is a Flux provides the ability to operate on paginated REST response of type {@link PagedResponse}
	* This type is a {@link Flux} that provides the ability to operate on paginated REST response of type {@link PagedResponse}

Paged Flux abstractions #6465

Paged Flux abstractions #6465

Conversation

anuchandy commented Nov 21, 2019

anuchandy commented Dec 11, 2019 • edited Loading

alzimmermsft commented Dec 11, 2019

anuchandy commented Dec 12, 2019

srnagar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JonathanGiles left a comment

Choose a reason for hiding this comment

anuchandy commented Jan 17, 2020

hemanttanwar left a comment

Choose a reason for hiding this comment

anuchandy commented Jan 17, 2020

anuchandy commented Dec 11, 2019 •

edited

Loading