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

[general] Documentation on page_size is very minimal. #6207

Closed
Edo-A opened this issue Oct 15, 2018 · 2 comments
Closed

[general] Documentation on page_size is very minimal. #6207

Edo-A opened this issue Oct 15, 2018 · 2 comments
Assignees
Labels
api: core type: docs Improvement to the documentation for an API.

Comments

@Edo-A
Copy link

Edo-A commented Oct 15, 2018

The explanation of page_size is extremely limited.
Example from the logging API:
maximum number of entries to return, If not passed, defaults to a value set by the API.

If you're familiar with gRPC then this makes sense, but for newcomers this is extremely limited information. Simple wording change can make a lot of difference here IMHO. Looking at this example from the Cloud Spanner Javadocs shows a slight change, but makes it easier to understand:
Specifying this will cause the list operations to fetch at most this many records in a page.

On a semi-related note, having usage examples would also clarify the usage a lot, but this can be said for all methods in the whole python library.

@tseaver tseaver added api: core type: docs Improvement to the documentation for an API. labels Oct 15, 2018
@Edo-A
Copy link
Author

Edo-A commented Oct 16, 2018

Another thing that needs attention as well is the related page_token. Wording is actually more confusing for this one: opaque marker?

The methods link to the page describing the API which has a much better description. Copying those into the lib itself would actually solve the issue without actually having to write up something new. Compare the following:

• page_size
From the lib:
maximum number of entries to return, If not passed, defaults to a value set by the API.
From the API description:
Optional. The maximum number of results to return from this request. Non-positive values are ignored. The presence of nextPageToken in the response indicates that more results might be available.

• page_token
From the lib:
opaque marker for the next "page" of entries. If not passed, the API will return the first page of entries.
From the API description:
Optional. If present, then retrieve the next batch of results from the preceding call to this method. pageToken must be the value of nextPageToken from the previous response. The values of other method parameters should be identical to those in the previous call.

@tseaver tseaver self-assigned this Dec 4, 2018
@tseaver
Copy link
Contributor

tseaver commented Dec 4, 2018

@Edo-A note that we actually don't really expect / want users to pass either value explicitly in normal usage: the list_foo methods return iterators which provide a much better surface for processing results, either item-by-item or page-at-a-time.

page_size might be useful to override the APIs default in some very unusual cases (but it seems dubious).

I'm going to say we should docs-deprecate page_token everywhere in non-generated code, with a note to use the iterator's pages property if paging is desired.

I'm going to scope this one to ignore occurrences of page_size / page_token in GAPIC- or protoc-generated code.

For page_size:

$ grep -rl page_size docs/_build/html/_modules/ | grep -v gapic | grep -v "\<proto\>" | sort
docs/_build/html/_modules/google/cloud/bigquery/client.html
docs/_build/html/_modules/google/cloud/bigquery/dbapi/cursor.html
docs/_build/html/_modules/google/cloud/bigquery/table.html
docs/_build/html/_modules/google/cloud/firestore_v1beta1/document.html
docs/_build/html/_modules/google/cloud/logging/client.html
docs/_build/html/_modules/google/cloud/logging/logger.html
docs/_build/html/_modules/google/cloud/monitoring_v3/query.html
docs/_build/html/_modules/google/cloud/resource_manager/client.html
docs/_build/html/_modules/google/cloud/runtimeconfig/config.html
docs/_build/html/_modules/google/cloud/spanner_v1/client.html
docs/_build/html/_modules/google/cloud/spanner_v1/instance.html

For page_token:

$ grep -rl page_token docs/_build/html/_modules/ | grep -v gapic | grep -v "\<proto\>" | sort
docs/_build/html/_modules/google/api_core/operations_v1/operations_client.html
docs/_build/html/_modules/google/api_core/page_iterator.html
docs/_build/html/_modules/google/cloud/bigquery/client.html
docs/_build/html/_modules/google/cloud/bigquery/query.html
docs/_build/html/_modules/google/cloud/bigquery/table.html
docs/_build/html/_modules/google/cloud/datastore/client.html
docs/_build/html/_modules/google/cloud/datastore/query.html
docs/_build/html/_modules/google/cloud/dns/client.html
docs/_build/html/_modules/google/cloud/dns/zone.html
docs/_build/html/_modules/google/cloud/logging/client.html
docs/_build/html/_modules/google/cloud/logging/logger.html
docs/_build/html/_modules/google/cloud/runtimeconfig/config.html
docs/_build/html/_modules/google/cloud/spanner_v1/client.html
docs/_build/html/_modules/google/cloud/spanner_v1/instance.html
docs/_build/html/_modules/google/cloud/storage/bucket.html
docs/_build/html/_modules/google/cloud/storage/client.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: core type: docs Improvement to the documentation for an API.
Projects
None yet
Development

No branches or pull requests

2 participants