feat(diregapic)(pagination): support map handle for DIREGAPIC Pagination #1052

summer-ji-eng · 2021-07-09T20:03:19Z

The DIREGAPIC pagination design suggests that the client libraries should return the whole contents of the dictionary key as a single resource being paginated. In the TypeScript implementation, we use tuple as representation.

Next TODO: Update client library surface of pagination response iterator type. Changes in gapic-generator-typescript

On the client side, the usage is

const mapIterables = await client.aggregatedListAsync({
  project,
  region,
  maxResults,
})

for await (const [key, value] of mapIterables) {
  console.log("aggregatedListAsync iterable key: ", key);
  console.log("aggregatedListAsync iterable value: ", value);
  require('fs').appendFileSync("/tmp/out.json", JSON.stringify({key: key, value: value}, null, '  ') + ',\r\n');
}

The output:

{
  "key": "global",
  "value": {
    "warning": {
      "code": "NO_RESULTS_ON_PAGE",
      "data": [
        {
          "key": "scope",
          "value": "global"
        }
      ],
      "message": "There are no results for scope 'global' on this page."
    }
  }
},
{
  "key": "regions/us-central1",
  "value": {
    "warning": {
      "code": "NO_RESULTS_ON_PAGE",
      "data": [
        {
          "key": "scope",
          "value": "regions/us-central1"
        }
      ],
      "message": "There are no results for scope 'regions/us-central1' on this page."
    }
  }
},
{
  "key": "regions/us-central2",
  "value": {
    "warning": {
      "code": "NO_RESULTS_ON_PAGE",
      "data": [
        {
          "key": "scope",
          "value": "regions/us-central2"
        }
      ],
      "message": "There are no results for scope 'regions/us-central2' on this page."
    }
  }
},
{
  "key": "regions/europe-west1",
  "value": {
    "warning": {
      "code": "NO_RESULTS_ON_PAGE",
      "data": [
        {
          "key": "scope",
          "value": "regions/europe-west1"
        }
      ],
      "message": "There are no results for scope 'regions/europe-west1' on this page."
    }
  }
},
{
  "key": "regions/us-west1",
  "value": {
    "warning": {
      "code": "NO_RESULTS_ON_PAGE",
      "data": [
        {
          "key": "scope",
          "value": "regions/us-west1"
        }
      ],
      "message": "There are no results for scope 'regions/us-west1' on this page."
    }
  }
},
{
  "key": "regions/asia-east1",
  "value": {
    "warning": {
      "code": "NO_RESULTS_ON_PAGE",
      "data": [
        {
          "key": "scope",
          "value": "regions/asia-east1"
        }
      ],
      "message": "There are no results for scope 'regions/asia-east1' on this page."
    }
  }
},
{
  "key": "regions/us-east1",
  "value": {
    "addresses": [
      {
        "id": "5011754511478056813",
        "kind": "compute#address",
        "name": "test-address-0",
        "creationTimestamp": "2021-05-28T23:04:50.044-07:00",
        "region": "https://www.googleapis.com/compute/v1/projects/gapic-images/regions/us-east1",
        "status": "RESERVED",
        "addressType": "EXTERNAL",
        "description": "",
        "selfLink": "https://www.googleapis.com/compute/v1/projects/gapic-images/regions/us-east1/addresses/test-address-0",
        "address": "35.237.96.161",
        "networkTier": "PREMIUM"
      },
      {
        "id": "1036412484008568684",
        "kind": "compute#address",
        "name": "test-address-1",
        "creationTimestamp": "2021-05-28T23:04:51.044-07:00",
        "region": "https://www.googleapis.com/compute/v1/projects/gapic-images/regions/us-east1",
        "status": "RESERVED",
        "addressType": "EXTERNAL",
        "description": "",
        "selfLink": "https://www.googleapis.com/compute/v1/projects/gapic-images/regions/us-east1/addresses/test-address-1",
        "address": "35.196.61.221",
        "networkTier": "PREMIUM"
      },
      {
        "id": "6976094650828089196",
        "kind": "compute#address",
        "name": "test-address-2",
        "creationTimestamp": "2021-05-28T23:04:51.890-07:00",
        "region": "https://www.googleapis.com/compute/v1/projects/gapic-images/regions/us-east1",
        "status": "RESERVED",
        "addressType": "EXTERNAL",
        "description": "",
        "selfLink": "https://www.googleapis.com/compute/v1/projects/gapic-images/regions/us-east1/addresses/test-address-2",
        "address": "35.190.159.73",
        "networkTier": "PREMIUM"
      },
      {
        "id": "311669785402042219",
        "kind": "compute#address",
        "name": "test-address-3",
        "creationTimestamp": "2021-05-28T23:04:52.769-07:00",
        "region": "https://www.googleapis.com/compute/v1/projects/gapic-images/regions/us-east1",
        "status": "RESERVED",
        "addressType": "EXTERNAL",
        "description": "",
        "selfLink": "https://www.googleapis.com/compute/v1/projects/gapic-images/regions/us-east1/addresses/test-address-3",
        "address": "35.243.170.233",
        "networkTier": "PREMIUM"
      },
      {
        "id": "5308273027754780522",
        "kind": "compute#address",
        "name": "test-address-4",
        "creationTimestamp": "2021-05-28T23:04:53.619-07:00",
        "region": "https://www.googleapis.com/compute/v1/projects/gapic-images/regions/us-east1",
        "status": "RESERVED",
        "addressType": "EXTERNAL",
        "description": "",
        "selfLink": "https://www.googleapis.com/compute/v1/projects/gapic-images/regions/us-east1/addresses/test-address-4",
        "address": "34.74.32.45",
        "networkTier": "PREMIUM"
      }
    ]
  }
},
{
  "key": "regions/asia-northeast1",
  "value": {
    "warning": {
      "code": "NO_RESULTS_ON_PAGE",
      "data": [
        {
          "key": "scope",
          "value": "regions/asia-northeast1"
        }
      ],
      "message": "There are no results for scope 'regions/asia-northeast1' on this page."
    }
  }
},
...

summer-ji-eng · 2021-07-10T05:49:05Z

src/paginationCalls/pageDescriptor.ts

+              // For pagination response with protobuf map type, use tuple as representation.
+              if (result && !Array.isArray(result)) {
+                for (const [key, value] of Object.entries(result)) {
+                  cache.push([key, value]);


I am not sure if we need to define a type for this tuple. @alexander-fenster what do you suggest the type here? My guess here is that we could define a MapResponseTuple

type MapResponseTuple = [String, ??(not sure)]

It would be [string, ResponseType], and the cache should be defined as ResponseType | [string, ResponseType].

(I think having just a ResponseType which is basically any object is OK)

test/unit/pagedIteration.ts

alexander-fenster

LGTM with comments

vam-google

I'll let Alex to comment on the type-script fine grain details, but overall the change looks good to me! look simple & robust and gets the job done.

summer-ji-eng · 2021-07-12T23:47:12Z

src/paginationCalls/pageDescriptor.ts

@@ -143,7 +143,7 @@ export class PageDescriptor implements Descriptor {
    const asyncIterable = {
      [Symbol.asyncIterator]() {
        let nextPageRequest: RequestType | null | undefined = request;
-        const cache: {}[] = [];
+        const cache: Array<ResponseType | [string, ResponseType]> = [];


Another option
const cache: (ResponseType | [string, ResponseType]) = [],
Base on my search Array<T> vs T[] are quite similar, [] is shorthand way, I choose Array<T> because it is more readable with multiple types in my opinion.
Let me know if you have any concern. @alexander-fenster Thanks.

They are similar, and gts forces us to use Array<> for complex types and [] for simple types. You did it correctly here.

summer-ji-eng

@alexander-fenster Made change, and please take another quick look, and I will merge the PR.

Thanks a lot for review my PRs.

summer-ji-eng requested a review from a team as a code owner July 9, 2021 20:03

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 9, 2021

summer-ji-eng added the DoNotReview label Jul 9, 2021

summer-ji-eng requested review from vam-google and alexander-fenster July 10, 2021 05:42

summer-ji-eng commented Jul 10, 2021

View reviewed changes

summer-ji-eng removed the DoNotReview label Jul 10, 2021

summer-ji-eng mentioned this pull request Jul 12, 2021

feat(diregapic)(pagination): support paging map response type googleapis/gapic-generator-typescript#922

Merged

alexander-fenster reviewed Jul 12, 2021

View reviewed changes

test/unit/pagedIteration.ts Show resolved Hide resolved

summer-ji-eng force-pushed the pagination_map_handle branch from 9f3c271 to b1c6128 Compare July 12, 2021 18:02

summer-ji-eng added 3 commits July 12, 2021 11:30

add implementation and test

47af0c5

remove console.log

759baa9

add type for tuple

2a2c7b7

summer-ji-eng force-pushed the pagination_map_handle branch from b1c6128 to 2a2c7b7 Compare July 12, 2021 18:32

alexander-fenster approved these changes Jul 12, 2021

View reviewed changes

vam-google approved these changes Jul 12, 2021

View reviewed changes

Add detail resource check

6b2590a

summer-ji-eng requested a review from alexander-fenster July 12, 2021 23:43

summer-ji-eng commented Jul 12, 2021

View reviewed changes

alexander-fenster approved these changes Jul 13, 2021

View reviewed changes

alexander-fenster merged commit faab4c6 into master Jul 13, 2021

alexander-fenster deleted the pagination_map_handle branch July 13, 2021 16:05

summer-ji-eng changed the title ~~feat: support map handle for DIREGAPIC Pagination~~ feat: [diregapic][pagination]support map handle for DIREGAPIC Pagination Sep 9, 2021

summer-ji-eng changed the title ~~feat: [diregapic][pagination]support map handle for DIREGAPIC Pagination~~ feat(diregapic)(pagination): support map handle for DIREGAPIC Pagination Sep 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(diregapic)(pagination): support map handle for DIREGAPIC Pagination #1052

feat(diregapic)(pagination): support map handle for DIREGAPIC Pagination #1052

summer-ji-eng commented Jul 9, 2021 •

edited

summer-ji-eng Jul 10, 2021

alexander-fenster Jul 12, 2021

alexander-fenster Jul 12, 2021

alexander-fenster left a comment

vam-google left a comment

summer-ji-eng Jul 12, 2021

alexander-fenster Jul 13, 2021

summer-ji-eng left a comment

feat(diregapic)(pagination): support map handle for DIREGAPIC Pagination #1052

feat(diregapic)(pagination): support map handle for DIREGAPIC Pagination #1052

Conversation

summer-ji-eng commented Jul 9, 2021 • edited

summer-ji-eng Jul 10, 2021

Choose a reason for hiding this comment

alexander-fenster Jul 12, 2021

Choose a reason for hiding this comment

alexander-fenster Jul 12, 2021

Choose a reason for hiding this comment

alexander-fenster left a comment

Choose a reason for hiding this comment

vam-google left a comment

Choose a reason for hiding this comment

summer-ji-eng Jul 12, 2021

Choose a reason for hiding this comment

alexander-fenster Jul 13, 2021

Choose a reason for hiding this comment

summer-ji-eng left a comment

Choose a reason for hiding this comment

summer-ji-eng commented Jul 9, 2021 •

edited