-
Notifications
You must be signed in to change notification settings - Fork 97
feat: support Iam_service in Gax #762
Conversation
Codecov Report
@@ Coverage Diff @@
## master googleapis/gapic-generator-typescript#762 +/- ##
=======================================
Coverage 88.23% 88.23%
=======================================
Files 47 47
Lines 7965 7965
Branches 488 488
=======================================
Hits 7028 7028
Misses 934 934
Partials 3 3 Continue to review full report at Codecov.
|
|
Current problem:
Questions: can we expose |
Easy! As for exposing - yes, sure, we can expose whatever we need there. |
|
@alexander-fenster yes I removed all dependencies and build files, but still it passes the tests :( |
alexander-fenster
left a comment
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.
LGTM with nits. Would be great to have a system test covering this functionality, but I'm not sure how hard would it be to get one.
|
TO DO: once KMS is migrated using |
alexander-fenster
left a comment
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.
LGTM!
Source-Repo: googleapis/gax-nodejs Source-Sha: a4c0dd3 Source-Link: a4c0dd3 Author: Xiaozhen Liu <xiaozhenliu@google.com> Date: Thu Apr 9 16:43:17 2020 -0700 Original-Commit-Message: feat: support Iam_service in Gax (#762) * add IAM service * add unit test * only iam * sinon, reslove circular dependency * test * test * test * copyright * test * test * test * test * debug * fix cannot find module failure * tests pass! * sample/system-test pass locally * unit tests pass without IAM serive test * fix: proto copy * proper unit test * copyright * export IAM protos * init before calling IAM service * compile-iam-protos & rename protosTypes * comments * import iamClient for unit test * update iam protos by compile script * add license to protos * move expoted common interface to separat file * header for clientInterface.ts * accident * export IamClient interface * using Callback, unit test * lint Co-authored-by: Alexander Fenster <fenster@google.com>
Source-Repo: googleapis/gax-nodejs Source-Sha: a4c0dd3 Source-Link: a4c0dd3 Author: Xiaozhen Liu <xiaozhenliu@google.com> Date: Thu Apr 9 16:43:17 2020 -0700 Original-Commit-Message: feat: support Iam_service in Gax (#762) * add IAM service * add unit test * only iam * sinon, reslove circular dependency * test * test * test * copyright * test * test * test * test * debug * fix cannot find module failure * tests pass! * sample/system-test pass locally * unit tests pass without IAM serive test * fix: proto copy * proper unit test * copyright * export IAM protos * init before calling IAM service * compile-iam-protos & rename protosTypes * comments * import iamClient for unit test * update iam protos by compile script * add license to protos * move expoted common interface to separat file * header for clientInterface.ts * accident * export IamClient interface * using Callback, unit test * lint Co-authored-by: Alexander Fenster <fenster@google.com>
Supporting IAM service in Gax-nodejs.
Nodejs-kmsandnodejs-pubsubare the two APIs that require IAM Client to be part of the methods [getIamPolicy, setIamPolicy, testIamPermission].Monolith generator has a
reroute_to_grpc_interfacesetting in GAPIC yaml to include extra services when generating client libraries. While this is not supported in micro-generator today. And IAM service is the only one referenced in that way but it does not have a generated client library [nodejs-IAM] for easy use. This is a pain point for every micro-generator and it makes post-processing script (synth.py in Typescript Microgenerator) really messy.related issue: https://github.com/googleapis/gapic-generator-typescript/issues/315