GRPC API Review Changes

[tegefaulkes]

Description

This PR is for changes relating to the API review discussion.

Issues Fixed

Fixes #249

Tasks

1. - Review the proto3 guide, naming standard with reference to the proto files, look for ways to abstract or clean up the data
   • - Version the Client.proto and Agent.proto with version names and test out multiple-version services, and find out what the client does when the version wanted is not available
     • - create a new version for a service in the protobuf definitions.
     • - run both services at the same time.
     • - test communicating with a V1 service.
     • - test communicating with a v2 service.
     • - test communicating with a "removed" service.
   • - Test out grpc reflection for versioning if necessary
   • - Update all the primitive types, and any usage of 64 bit int should have the string hint
   • - Add some benchmarks to the grpc, and check if MAX_CONCURRENT_CONNECTIONS is used
   • - Clean up types Wrapper methods in GRPCClientClient to get parameter types of the wrapped GRPC call #200 or gRPC abstractions based on new grpc libraries
   • - Consider how to shutdown the grpc server and terminate all client connections as well
2. - Figure out CQRS, pagination and streaming
   • - Clean up js-pagination client and server side utilities
   • - Pagination on unary calls, and streaming calls that can be initialised with a pagination parameters (which would enable some sort of CQRS and event sourcing concepts)
3. - Review in reference to authentication and sessions
4. - Check the HTTP API and web gateway compatibility
5. - Deal with the fact that the new grpc-js 1.4.1 version has some incompatibility: TypeError: http2Server.on is not a function.

Final checklist

• Domain specific tests
• Full tests
• Updated inline-comment documentation
• Lint fixed
• Squash and rebased
• Sanity check the final build

[CMCDragonkai]

@tegefaulkes please copy the tasks from the #249 into this and flesh them out as well.

[tegefaulkes]

OK, so the way we can include the google.protobuf types is pretty simple. the definitions are bundled with the protoc releases. for now i'm copying the google folder from the protoc release in src/proto/schema/google. from here we can just import in in a proto file with `import "google/proto/message.proto".

[tegefaulkes]

With the Any message type we can embed any message in that field. to create an Any message we can use the .pack() method. we provided it serialized binary form of a message echoMessage.seralizeBinary() and name to identify the message. 'common.EchoMessage'. we can retrieve the message using the .unpack() method. we provide it the DeseralizeBinary() for the expected message and the identifier name for the message. If the name matches the message name then the message is returned otherwise null is returned.

  const anyMessage = new Any();
  const echoMessage = new messages.common.EchoMessage();
  anyMessage.pack(echoMessage.serializeBinary(), 'common.EchoMessage');

  const test2 = anyMessage.unpack(
    messages.common.EchoMessage.deserializeBinary,
    'common.EchoMessage',
  );
  console.log(test2); // Is a EchoMessage
  const test3 = anyMessage.unpack(
    messages.common.StatusMessage.deserializeBinary,
    'common.StatusMessage',
  );
  console.log(test3); // Is null

The only use for this that comes to mind is a wrapper message for the NAT relay.

[tegefaulkes]

Timestamp message is pretty simple. we can use it to send date info without converting to nanoseconds first.

  const timeStampMessage = new Timestamp();
  timeStampMessage.fromDate(new Date());

  console.log(timeStampMessage.getNanos());
  console.log(timeStampMessage.getSeconds());
  console.log(timeStampMessage.toDate());

[CMCDragonkai]

Our JS dates don't have nanoseconds, only up to milliseconds.

…

On 10/28/21 2:00 PM, Brian Botha wrote: |Timestamp| message is pretty simple. we can use it to send date info without converting to nanoseconds first. const timeStampMessage = new Timestamp(); timeStampMessage.fromDate(new Date()); console.log(timeStampMessage.getNanos()); console.log(timeStampMessage.getSeconds()); console.log(timeStampMessage.toDate()); — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#275 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAE4OHLCBSWGB7EMZ33ZRV3UJC4DTANCNFSM5GZD7JKA>.

[tegefaulkes]

The Struct type allows us to send primitive POJOs. this might be pretty useful.

  const testObject = {
    1: 1,
    "two": 2,
    "three": "Three",
  }
  const structMessage = Struct.fromJavaScript(testObject);

  console.log(structMessage.toJavaScript());
  // { '1': 1, three: 'Three', two: 2 }

[tegefaulkes]

for the service versioning I need to..

1. create a new version for a service in the protobuf definitions.
2. run both services at the same time.
3. test communicating with a V1 service.
4. test communicating with a v2 service.
5. test communicating with a "removed" service.

I occurs to me that we only really need to version the Agent service. The client is bundled with the agent so the service the use to communicate should match. However different versions of nodes should be able to communicate with each other.

[CMCDragonkai]

I think a single service version is enough for both client and agent. Both client and service should be using the same version integer.

On 29 October 2021 10:36:06 am AEDT, Brian Botha ***@***.***> wrote: for the service versioning I need to.. 1. create a new version for a service in the protobuf definitions. 2. run both services at the same time. 3. test communicating with a V1 service. 4. test communicating with a v2 service. 5. test communicating with a "removed" service. I occurs to me that we only really need to version the Agent service. The client is bundled with the agent so the service the use to communicate should match. However different versions of nodes should be able to communicate with each other. -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #275 (comment)

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[CMCDragonkai]

No the version number is only incremented on breaking changes. Meaning most of the time we will ensure backwards compatibility. This means when the version mismatch it means there is no possible communication.

On 29 October 2021 10:36:06 am AEDT, Brian Botha ***@***.***> wrote: for the service versioning I need to.. 1. create a new version for a service in the protobuf definitions. 2. run both services at the same time. 3. test communicating with a V1 service. 4. test communicating with a v2 service. 5. test communicating with a "removed" service. I occurs to me that we only really need to version the Agent service. The client is bundled with the agent so the service the use to communicate should match. However different versions of nodes should be able to communicate with each other. -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #275 (comment)

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[tegefaulkes]

Re-based on master.

[tegefaulkes]

I split off part of the tasks into it's own issue at #279 and PR at #280.

[CMCDragonkai]

PR description to be updated since #280 got merged. Should also clear this PR and start off from master if there are no useful changes beyond what was done for #280.

[CMCDragonkai]

@tegefaulkes before we forget what this PR was doing, I'm guessing the changes here were all prototyping and can be squashed into one to rebase?

[tegefaulkes]

I think the only changes this has was prototyping using the google protobuf messages and versioning. AFAIK those changes already exist in the master branch. It can be squashed and re-based. Keep in mind there are a lot of conflicts with importing messages.

[CMCDragonkai]

Going to close this for now, as the aim of #249 has changed to focus on replacing GRPC itself with something more generic. Plus this code is wildly outdated now.