rdblue commented on code in PR #9872:
URL: https://github.com/apache/iceberg/pull/9872#discussion_r1513322269


##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1610,13 +1610,19 @@ components:
 
     PageToken:
       description:
-        An opaque token which allows clients to make use of pagination for a 
list API (e.g. ListTables). 
-        Clients will initiate the first paginated request by sending an empty 
`pageToken` e.g. `GET /tables?pageToken` or `GET /tables?pageToken=`
-        signaling to the service that the response should be paginated.
-
-        Servers that support pagination will recognize `pageToken` and return 
a `next-page-token` in response if there are more results available.
-        After the initial request, it is expected that the value of 
`next-page-token` from the last response is used in the subsequent request.
-        Servers that do not support pagination will ignore `next-page-token` 
and return all results.
+        An opaque token that allows clients to make use of pagination for a 
list APIs 
+        (e.g. ListTables). Clients may initiate the first paginated request by 
sending an empty 
+        query parameter `pageToken` (e.g. `GET /tables?pageToken` or `GET 
/tables?pageToken=`) 
+        requesting a paginated response from the server.
+
+        Servers that support pagination will identify the `pageToken` 
parameter and return a 
+        `next-page-token` in the response if there are more results available. 
 After the initial 
+        request, the value of `next-page-token` from each response is used as 
the parameter value 
+        for the next request.  An empty `next-page-token` in the response 
signals that pagination
+        is complete.
+        
+        Servers that do not support pagination will ignore the `pageToken` 
parameter and return all 
+        results in a single response.

Review Comment:
   In this case, `next-page-token` is going to be missing, so there's an 
argument for not sending it at all.



##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1610,13 +1610,19 @@ components:
 
     PageToken:
       description:
-        An opaque token which allows clients to make use of pagination for a 
list API (e.g. ListTables). 
-        Clients will initiate the first paginated request by sending an empty 
`pageToken` e.g. `GET /tables?pageToken` or `GET /tables?pageToken=`
-        signaling to the service that the response should be paginated.
-
-        Servers that support pagination will recognize `pageToken` and return 
a `next-page-token` in response if there are more results available.
-        After the initial request, it is expected that the value of 
`next-page-token` from the last response is used in the subsequent request.
-        Servers that do not support pagination will ignore `next-page-token` 
and return all results.
+        An opaque token that allows clients to make use of pagination for a 
list APIs 
+        (e.g. ListTables). Clients may initiate the first paginated request by 
sending an empty 
+        query parameter `pageToken` (e.g. `GET /tables?pageToken` or `GET 
/tables?pageToken=`) 

Review Comment:
   I don't think there should be an "or" unless the two options are broadly 
considered completely equivalent. I'd prefer:
   > (e.g., `GET /v1/namespaces/ns/tables?pageToken=`)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to