Conversation
|
@alexander-fenster, I think the failing tests are due to gapic-tools not recognizing the field_info.proto. Once we publish a new version of gax with the new version of nodejs-proto-files, I can also update gapic-tools and I think it will just work. |
|
Let me review this after #1503 gets in. |
|
|
||
| import * as protos from '../../protos/protos'; | ||
| import jsonProtos = require('../../protos/protos.json'); | ||
| import * as crypto from 'crypto'; |
There was a problem hiding this comment.
This immediately disqualifies the library from the browser or any other non-Node.js usage. One way to solve this would be to move everything that needs crypto to google-gax and export it from there, and in gax, make it similar to https://github.com/googleapis/google-auth-library-nodejs/tree/main/src/crypto to use either Node.js crypto or window.crypto.subtle, whichever is available.
Also, I don't see any crypto usage in this particular file, which probably means that this import should be conditional.
| ]>|void { | ||
| request = request || {}; | ||
| {% for field in method.autoPopulatedFields %}if (!request.{{ field.toCamelCase() }}) { | ||
| request.{{ field.toCamelCase() }} = crypto.randomUUID(); |
There was a problem hiding this comment.
To make it compatible with browser, we have two options: either use uuid module instead of crypto (which will force us make uuid a dependency for all libraries), or - probably better - export a createUUID() function from gax, and have it there instead.
…cript into addUUID
typescript/src/schema/proto.ts
Outdated
|
|
||
| export interface ServiceDescriptorProto | ||
| extends protos.google.protobuf.IServiceDescriptorProto { | ||
| getAutoPopulatedFields: MethodDescriptorProto[]; |
There was a problem hiding this comment.
Is this field actually used anywhere? I see the function getAutoPopulatedFields but I don't see if this added field is accessed anywhere.
| ]>|void { | ||
| request = request || {}; | ||
| if (!request.requestId) { | ||
| request.requestId = gax.makeUUID(); |
There was a problem hiding this comment.
Could we have a generated unit test that would check that this code is actually executed? It would be similar to an existing test that makes a request, but in a stub, it can check that the field was indeed assigned to some UUID. Similar to
.
No description provided.