Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

feat!: new generated version of compute API #537

Merged
merged 28 commits into from
Aug 11, 2021
Merged

feat!: new generated version of compute API #537

merged 28 commits into from
Aug 11, 2021

Conversation

alexander-fenster
Copy link
Contributor

@alexander-fenster alexander-fenster commented Jan 12, 2021

Generator: gapic-generator-typescript version: 2.2.0

Break Change
As we discuss in the email thread, we remove hand written features and will provide sample code.

Additional features:

  • Support Pagination for compute where it is not-AIP compliant
  • Detect transports
  • REST-encode requests
  • Default value
  • Pass auth in HTTP headers
    etc.

CLI command to generate nodejs-compute local:

$ bazel run //:gapic_generator_typescript -- -I $GOOGLEAPIS_DISCOVERY --output-dir /tmp/output $GOOGLEAPIS_DISCOVERY/google/cloud/compute/v1/compute.proto --package-name @google-cloud/compute --diregapic

Next: Update Bazel file to support auto-generation PR
Once it done, it is available using:
bazel build //google/cloud/compute/v1:compute-v1-nodejs

@product-auto-label product-auto-label bot added the api: compute Issues related to the googleapis/nodejs-compute API. label Jan 12, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jan 12, 2021
@snippet-bot
Copy link

snippet-bot bot commented Jan 12, 2021

Here is the summary of possible violations 😱

There are 5 possible violations for removing region tag in use.

The end of the violation section. All the stuff below is FYI purposes only.


Here is the summary of changes.

You are about to add 12 region tags.
You are about to delete 11 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@alexander-fenster alexander-fenster added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 12, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 12, 2021
@codecov
Copy link

codecov bot commented Jan 12, 2021

Codecov Report

Merging #537 (c0679ad) into master (7119ace) will decrease coverage by 2.44%.
The diff coverage is 97.20%.

❗ Current head c0679ad differs from pull request most recent head 28a3b20. Consider uploading reports for the commit 28a3b20 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #537      +/-   ##
==========================================
- Coverage   99.61%   97.16%   -2.45%     
==========================================
  Files          22       77      +55     
  Lines       11948    78901   +66953     
  Branches        0     3810    +3810     
==========================================
+ Hits        11902    76666   +64764     
- Misses         46     2162    +2116     
- Partials        0       73      +73     
Impacted Files Coverage Δ
src/v1/firewalls_client.ts 96.70% <ø> (ø)
src/v1/forwarding_rules_client.ts 97.16% <ø> (ø)
src/v1/global_addresses_client.ts 96.18% <ø> (ø)
src/v1/global_forwarding_rules_client.ts 96.96% <ø> (ø)
src/v1/global_network_endpoint_groups_client.ts 97.23% <ø> (ø)
src/v1/global_operations_client.ts 96.60% <ø> (ø)
src/v1/global_organization_operations_client.ts 95.76% <ø> (ø)
src/v1/health_checks_client.ts 97.09% <ø> (ø)
src/v1/images_client.ts 97.49% <ø> (ø)
src/v1/index.ts 100.00% <ø> (ø)
... and 141 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 154ee20...28a3b20. Read the comment docs.

@alexander-fenster alexander-fenster changed the title DO NOT MERGE: GAPIC client library feat!: DO NOT MERGE: GAPIC client library Jan 12, 2021
@alexander-fenster alexander-fenster added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jan 12, 2021
@bcoe bcoe mentioned this pull request Jan 12, 2021
4 tasks
@roikoren755
Copy link

roikoren755 commented Jan 16, 2021

@bcoe Picking up on the conversation from #534 -
I'm trying to run the equivalent of await new Compute().getVMs({ filter: 'some filter' }) from v2.
Trying the following code results in an error:

const client = new v1.InstancesClient();
const [instancesList] = await client.list({ filter: 'some string' }); // client.aggregatedList gives the same error...

The error is 13 INTERNAL: Received RST_STREAM with code 0. If I add { fallback: true } as parameter for the client constructor, the error changes to invalid wire type 4 at offset 1.

@@ -93,14 +115,7 @@ Samples are in the [`samples/`](https://github.com/googleapis/nodejs-compute/tre

| Sample | Source Code | Try it |
| --------------------------- | --------------------------------- | ------ |
| Create VM | [source code](https://github.com/googleapis/nodejs-compute/blob/master/samples/createVM.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-compute&page=editor&open_in_editor=samples/createVM.js,samples/README.md) |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could port a couple of the more interesting samples to the new API surface.

@MHDante
Copy link

MHDante commented Mar 15, 2021

Hi, @bcoe. When trying to use the 3.0.0-alpha.1 release, I ran into what looks like a proto file mismatch.

The following code:

import compute = require('@google-cloud/compute');


async function main(){
var instancesClient = new compute.InstancesClient({fallback: true}); 

const [list] = await instancesClient.list({})
console.log(list)

}

main().catch(e=> console.error(e))

yields the following error:


Error: invalid wire type 4 at offset 1
    at Reader.skipType (C:\Repos\pdp-deploy\node_modules\protobufjs\src\reader.js:377:19)
    at Type.Status$decode [as decode] (eval at Codegen (C:\Repos\pdp-deploy\node_modules\@protobufjs\codegen\index.js:50:33), <anonymous>:22:5)     
    at Type.decode_setup [as decode] (C:\Repos\pdp-deploy\node_modules\protobufjs\src\type.js:507:25)
    at FallbackErrorDecoder.decodeRpcStatus (C:\Repos\pdp-deploy\node_modules\google-gax\src\fallbackError.ts:70:37)
    at FallbackErrorDecoder.decodeErrorFromBuffer (C:\Repos\pdp-deploy\node_modules\google-gax\src\fallbackError.ts:92:42)
    at C:\Repos\pdp-deploy\node_modules\google-gax\src\fallback.ts:439:45
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

Is this path (automated generation of client libraries) still the path forward for your team? If so, let me know, and I'd be happy to give more feedback on iterations of 3.0.0.

@MHDante
Copy link

MHDante commented Mar 15, 2021

I have 2 pieces of feedback at the moment.

I think that the typing for the fallback field inside ClientOptions is limited to boolean | undefined. Whereas the documentation indicates passing in a 'rest' string. Adding this (and a handy // @ts-ignore) solves the issue I mentioned above, however, updating the typing would be preferable.

Failure messages that occur when passing in an incomplete object as a request into a method are not very informational.

This line failed:
const [list] = await instancesClient.list({})
despite the fact that all of the fields in IListInstancesRequest are optional.

I got the message:
Error: Cannot build HTTP request for {}, method: List

My assumption is that it would use the default project and zone, but I then added the project and region as per the documentation. While this is on me for not RTFM, It would be useful for the error to display the required fields.

After this, the sample was usable. I'm quite enjoying some of the new ergonomics, so thank you for your effort!

@generated-files-bot
Copy link

generated-files-bot bot commented Apr 29, 2021

Warning: This pull request is touching the following templated files:

@google-cla
Copy link

google-cla bot commented Apr 29, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Apr 29, 2021
@alexander-fenster alexander-fenster added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Apr 29, 2021
@google-cla
Copy link

google-cla bot commented Apr 29, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot removed the cla: yes This human has signed the Contributor License Agreement. label Apr 29, 2021
@google-cla
Copy link

google-cla bot commented Jul 23, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

* generate paging version

* 🦉 Updates from OwlBot

See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
@google-cla
Copy link

google-cla bot commented Jul 30, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla
Copy link

google-cla bot commented Aug 6, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla
Copy link

google-cla bot commented Aug 6, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla
Copy link

google-cla bot commented Aug 11, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla
Copy link

google-cla bot commented Aug 11, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@summer-ji-eng summer-ji-eng removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Aug 11, 2021
@summer-ji-eng summer-ji-eng marked this pull request as ready for review August 11, 2021 20:40
@summer-ji-eng summer-ji-eng requested a review from a team as a code owner August 11, 2021 20:40
@summer-ji-eng summer-ji-eng removed the cla: no This human has *not* signed the Contributor License Agreement. label Aug 11, 2021
* tests: add pagination tests

* tests: lower max results
@google-cla
Copy link

google-cla bot commented Aug 11, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no This human has *not* signed the Contributor License Agreement. label Aug 11, 2021
@summer-ji-eng summer-ji-eng removed the cla: no This human has *not* signed the Contributor License Agreement. label Aug 11, 2021
@bcoe bcoe added the cla: yes This human has signed the Contributor License Agreement. label Aug 11, 2021
@bcoe bcoe changed the title feat!: DO NOT MERGE: GAPIC client library feat!: new generated version of compute API Aug 11, 2021
@bcoe bcoe merged commit 4023676 into master Aug 11, 2021
@bcoe bcoe deleted the diregapic branch August 11, 2021 22:33
@release-please release-please bot mentioned this pull request Aug 11, 2021
@bcoe bcoe restored the diregapic branch August 11, 2021 22:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api: compute Issues related to the googleapis/nodejs-compute API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants