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

feat(cmd-api-server): support grpc web services #1189 #1190

Merged
merged 3 commits into from
Aug 27, 2021

Conversation

petermetz
Copy link
Member

@petermetz petermetz commented Aug 9, 2021

Author: Peter Somogyvari [email protected]
Date: Tue Aug 10 21:00:26 2021 -0700

feat(cmd-api-server): support grpc web services protocolbuffers/protobuf#1189

Primary change:

The API server now contains a gRPC server in addition to what it
had before (HTTP+SocketIO servers) so that through this it can
provide the opportunity for plugins to expose their gRPC services
via the Cactus API server. It is possible for plugins to serve
their requests on multiple protocols at the same time which means
that this is a big step towards our goal of being properly multi-
protocol capable.

Secondary change(s):

  1. Custom protobuf-schema openapi generator template added because
    of this issue: fix: import public causes AssertionError thesayyn/protoc-gen-ts#82
    (we override the stock template to not use the public keyword)

TODO:

  1. Implement streaming healthcheck endpiont with gRPC

  2. Allow plugins to hook in their own gRPC service implementations.

Fixes protocolbuffers/protobuf#1189

Signed-off-by: Peter Somogyvari [email protected]

@petermetz
Copy link
Member Author

Error: Cannot find module './generated/proto/protoc-gen-ts/services/default_service_grpc_pb'
Require stack:
- /home/runner/work/cactus/cactus/packages/cactus-cmd-api-server/dist/lib/main/typescript/api-server.js
- /home/runner/work/cactus/cactus/packages/cactus-cmd-api-server/dist/lib/main/typescript/public-api.js
- /home/runner/work/cactus/cactus/packages/cactus-cmd-api-server/dist/lib/main/typescript/index.js
- /home/runner/work/cactus/cactus/packages/cactus-test-api-client/src/test/typescript/integration/api-client-routing-node-to-node.test.ts
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:815:15)
    at Function.Module._load (internal/modules/cjs/loader.js:667:27)
    at Module.require (internal/modules/cjs/loader.js:887:19)
    at require (internal/modules/cjs/helpers.js:74:18)
    at Object.<anonymous> (/home/runner/work/cactus/cactus/packages/cactus-cmd-api-server/dist/lib/main/typescript/api-server.js:2:2645)
    at Module._compile (internal/modules/cjs/loader.js:999:30)
    at Module.replacementCompile (/home/runner/work/cactus/cactus/node_modules/append-transform/index.js:60:13)
    at Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Object.<anonymous> (/home/runner/work/cactus/cactus/node_modules/append-transform/index.js:64:4)
    at Module.load (internal/modules/cjs/loader.js:863:32)
    at Function.Module._load (internal/modules/cjs/loader.js:708:14)
    at Module.require (internal/modules/cjs/loader.js:887:19)
    at require (internal/modules/cjs/helpers.js:74:18)
    at Object.<anonymous> (/home/runner/work/cactus/cactus/packages/cactus-cmd-api-server/dist/lib/main/typescript/public-api.js:2:1839)
    at Module._compile (internal/modules/cjs/loader.js:999:30)
    at Module.replacementCompile (/home/runner/work/cactus/cactus/node_modules/append-transform/index.js:60:13)
# Subtest: packages/cactus-test-api-client/src/test/typescript/integration/api-client-routing-node-to-node.test.ts
    1..0 # no tests found
not ok 94 - packages/cactus-test-api-client/src/test/typescript/integration/api-client-routing-node-to-node.test.ts # time=6163.532ms
  ---
  env:
    TS_NODE_COMPILER_OPTIONS: "{}"
  file: packages/cactus-test-api-client/src/test/typescript/integration/api-client-routing-node-to-node.test.ts
  timeout: 3600000
  command: /opt/hostedtoolcache/node/12.22.3/x64/bin/node
  args:
    - -r
    - /home/runner/work/cactus/cactus/node_modules/ts-node/register/index.js
    - --max-old-space-size=4096
    - packages/cactus-test-api-client/src/test/typescript/integration/api-client-routing-node-to-node.test.ts
  stdio:
    - 0
    - pipe
    - 2
  cwd: /home/runner/work/cactus/cactus
  exitCode: 1
  ...

Bail out! packages/cactus-test-api-client/src/test/typescript/integration/api-client-routing-node-to-node.test.ts

@petermetz petermetz force-pushed the feat-1189 branch 2 times, most recently from 0638005 to 3df47d2 Compare August 11, 2021 20:08
@codecov-commenter
Copy link

codecov-commenter commented Aug 11, 2021

Codecov Report

Merging #1190 (be2d1cb) into main (02fa383) will decrease coverage by 0.33%.
The diff coverage is 58.94%.

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

@@            Coverage Diff             @@
##             main    protocolbuffers/protobuf#1190      +/-   ##
==========================================
- Coverage   72.18%   71.85%   -0.34%     
==========================================
  Files         272      278       +6     
  Lines        9772    10016     +244     
  Branches     1179     1241      +62     
==========================================
+ Hits         7054     7197     +143     
- Misses       2099     2157      +58     
- Partials      619      662      +43     
Impacted Files Coverage Δ
...ed/proto/protoc-gen-ts/services/default_service.ts 31.25% <31.25%> (ø)
...rated/proto/protoc-gen-ts/google/protobuf/empty.ts 40.00% <40.00%> (ø)
...main/typescript/common/determine-address-family.ts 42.85% <42.85%> (ø)
...ated/proto/protoc-gen-ts/models/memory_usage_pb.ts 63.38% <63.38%> (ø)
...o/protoc-gen-ts/models/health_check_response_pb.ts 65.38% <65.38%> (ø)
...script/web-services/grpc/grpc-server-api-server.ts 77.77% <77.77%> (ø)
...s-cmd-api-server/src/main/typescript/api-server.ts 86.19% <81.81%> (-0.59%) ⬇️
...erver/src/main/typescript/config/config-service.ts 75.00% <100.00%> (+0.58%) ⬆️
...s-cmd-api-server/src/main/typescript/public-api.ts 100.00% <100.00%> (ø)
... and 5 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 02fa383...5e141bb. Read the comment docs.

Copy link
Contributor

@jonathan-m-hamilton jonathan-m-hamilton left a comment

Choose a reason for hiding this comment

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

LGTM

Primary change:
--------------
The API server now contains a gRPC server in addition to what it
had before (HTTP+SocketIO servers) so that through this it can
provide the opportunity for plugins to expose their gRPC services
via the Cactus API server. It is possible for plugins to serve
their requests on multiple protocols at the same time which means
that this is a big step towards our goal of being properly multi-
protocol capable.

Secondary change(s):
-------------------
1. Custom protobuf-schema openapi generator template added because
of this issue: thesayyn/protoc-gen-ts#82
(we override the stock template to not use the public keyword)

TODO:
----

1. Implement streaming healthcheck endpiont with gRPC

2. Allow plugins to hook in their own gRPC service implementations.

Fixes hyperledger#1189

Signed-off-by: Peter Somogyvari <[email protected]>
@petermetz petermetz merged commit 4cace1d into hyperledger:main Aug 27, 2021
@petermetz petermetz deleted the feat-1189 branch August 27, 2021 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API_Server enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Objective-C: The GPBArrays should mark its "values" init param as nullable.
4 participants