-
Notifications
You must be signed in to change notification settings - Fork 53
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
[samplecode]fix: add request with default value in regular paged callable method #690
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -613,6 +613,14 @@ public final ListLogEntriesPagedResponse listLogEntries(ListLogEntriesRequest re | |
* | ||
* <pre>{@code | ||
* try (LoggingClient loggingClient = LoggingClient.create()) { | ||
* ListLogEntriesRequest request = | ||
* ListLogEntriesRequest.newBuilder() | ||
* .addAllResourceNames(new ArrayList<String>()) | ||
* .setFilter("filter-1274492040") | ||
* .setOrderBy("orderBy-1207110587") | ||
* .setPageSize(883849137) | ||
* .setPageToken("pageToken873572522") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setting page token in the initial request seems harmful, because before you start paginated call there is no page token (i.e. page token makes sense only for pages 2+, but the first page does not have page token). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI @miraleung |
||
* .build(); | ||
* while (true) { | ||
* ListLogEntriesResponse response = loggingClient.listLogEntriesCallable().call(request); | ||
* for (LogEntry element : response.getResponsesList()) { | ||
|
@@ -697,6 +705,11 @@ public final ListMonitoredResourceDescriptorsPagedResponse listMonitoredResource | |
* | ||
* <pre>{@code | ||
* try (LoggingClient loggingClient = LoggingClient.create()) { | ||
* ListMonitoredResourceDescriptorsRequest request = | ||
* ListMonitoredResourceDescriptorsRequest.newBuilder() | ||
* .setPageSize(883849137) | ||
* .setPageToken("pageToken873572522") | ||
* .build(); | ||
* while (true) { | ||
* ListMonitoredResourceDescriptorsResponse response = | ||
* loggingClient.listMonitoredResourceDescriptorsCallable().call(request); | ||
|
@@ -919,6 +932,13 @@ public final UnaryCallable<ListLogsRequest, ListLogsPagedResponse> listLogsPaged | |
* | ||
* <pre>{@code | ||
* try (LoggingClient loggingClient = LoggingClient.create()) { | ||
* ListLogsRequest request = | ||
* ListLogsRequest.newBuilder() | ||
* .setParent(LogName.ofProjectLogName("[PROJECT]", "[LOG]").toString()) | ||
* .setPageSize(883849137) | ||
* .setPageToken("pageToken873572522") | ||
* .addAllResourceNames(new ArrayList<String>()) | ||
* .build(); | ||
* while (true) { | ||
* ListLogsResponse response = loggingClient.listLogsCallable().call(request); | ||
* for (String element : response.getResponsesList()) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we assing something reasonable (i.e smaller, like 10 or 100) to these kind of fields (random huge number for page size is confusing and not realistic)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, we use hashcode of field' name. source code
IMO, smaller number like 10 make more sense in reality. And PageSize is fixed filed to be set for default request variable. Looks like it is doable in code. @miraleung what is your opinion?