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

generalize traversing message types for REST query parameters #14492

Open
scotthart opened this issue Jul 18, 2024 · 1 comment
Open

generalize traversing message types for REST query parameters #14492

scotthart opened this issue Jul 18, 2024 · 1 comment
Assignees
Labels
cpp: generator Issues related to the C++ micro-generator type: cleanup An internal cleanup or hygiene concern.

Comments

@scotthart
Copy link
Member

For Requests that have query parameters nested in messages, we need to synthesize qualified names for those parameters and include them in the URL.

for this example, the URL should be something like:

"https://bigquery.googleapis.com/bigquery/v2/projects/<project id>/queries/<job_id>?maxResults=10&formatOptions.useInt6stamp=false"
// Options for data format adjustments.
message DataFormatOptions {
  // Optional. Output timestamp as usec int64. Default is false.
  bool use_int64_timestamp = 1 [(google.api.field_behavior) = OPTIONAL];
}

// Request object of GetQueryResults.
message GetQueryResultsRequest {
  // Required. Project ID of the query job.
  string project_id = 1 [(google.api.field_behavior) = REQUIRED];

  // Required. Job ID of the query job.
  string job_id = 2 [(google.api.field_behavior) = REQUIRED];

  // Zero-based index of the starting row.
  google.protobuf.UInt64Value start_index = 3;

  // Page token, returned by a previous call, to request the next page of
  // results.
  string page_token = 4;

  // Maximum number of results to read.
  google.protobuf.UInt32Value max_results = 5;

  // Optional: Specifies the maximum amount of time, in milliseconds, that the
  // client is willing to wait for the query to complete. By default, this limit
  // is 10 seconds (10,000 milliseconds). If the query is complete, the
  // jobComplete field in the response is true. If the query has not yet
  // completed, jobComplete is false.
  //
  // You can request a longer timeout period in the timeoutMs field.  However,
  // the call is not guaranteed to wait for the specified timeout; it typically
  // returns after around 200 seconds (200,000 milliseconds), even if the query
  // is not complete.
  //
  // If jobComplete is false, you can continue to wait for the query to complete
  // by calling the getQueryResults method until the jobComplete field in the
  // getQueryResults response is true.
  google.protobuf.UInt32Value timeout_ms = 6;

  // The geographic location of the job. You must specify the location to run
  // the job for the following scenarios:
  //
  // * If the location to run a job is not in the `us` or
  //   the `eu` multi-regional location
  // * If the job's location is in a single region (for example,
  // `us-central1`)
  //
  // For more information, see
  // https://cloud.google.com/bigquery/docs/locations#specifying_your_location.
  string location = 7;

  // Optional. Output format adjustments.
  DataFormatOptions format_options = 8 [(google.api.field_behavior) = OPTIONAL];
}



@scotthart scotthart added type: cleanup An internal cleanup or hygiene concern. cpp: generator Issues related to the C++ micro-generator labels Jul 18, 2024
@dbolduc dbolduc self-assigned this Jul 18, 2024
@dbolduc
Copy link
Member

dbolduc commented Jul 24, 2024

Just to checkpoint progress before I go on a mini-vacation...

I have generator changes in a branch: https://github.com/dbolduc/google-cloud-cpp/tree/generator-rest-query-params-dev. It also addresses #10176.

The code looks right to my eye, but I haven't written tests. Golden unit tests are where we need to do the bulk of the testing. Existing integration tests should at least give us confidence things are no worse than before. They won't verify the new functionality though.

The PR is too big already. I will want to break it down:

  • No need for query parameters when body = "*"
  • Factor out the params argument to the rest_internal::Verb(...). This will be the majority of the line changes, although the PR should be trivial.
  • Factor out the accessor value adapter
  • Probably refactor to set up the algorithm witqh messages, repeated disabled. I will need to remember to leave in the Protobuf value processing, so there is not a regression.
  • Turn on messages and repeated fields

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp: generator Issues related to the C++ micro-generator type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

No branches or pull requests

2 participants