Implement Roughtime client pseudo-node #1086
Conversation
| DEFAULT_MIN_OVERLAPPING_INTERVALS, DEFAULT_SERVER_RETRIES, DEFAULT_TIMEOUT_SECONDS, | ||
| }, | ||
| }; | ||
|
|
There was a problem hiding this comment.
nit: I think we're mostly avoiding blank lines in use statements, and letting rustfmt handle the ordering (which will keep crate:: lines at the top I think).
| }, | ||
| OakStatus, | ||
| }; | ||
| use prost::Message as _; |
There was a problem hiding this comment.
nit: is the as _ needed? I couldn't immediately see a clashing Message.
There was a problem hiding this comment.
Removed. I thought it was needed to stop cargo from complaining about unused imports, but cargo did not complain after removing it.
| match invocation.receive_request(runtime) { | ||
| Ok(request) => { | ||
| if request.method_name != "/oak.roughtime.RoughtimeService/GetRoughtime" { | ||
| let message = format!("Unknwon method_name: {}", request.method_name); |
| let mut message = Vec::new(); | ||
| match reply.encode(&mut message) { | ||
| Ok(_) => { | ||
| let response = GrpcResponse { |
There was a problem hiding this comment.
nit: having both reply and response is a bit confusing; maybe rsp and grpc_rsp or somesuch?
oak/proto/roughtime_service.proto
Outdated
| } | ||
|
|
||
| // Interface exposed by the Roughtime client pseudo-Node to other nodes over a | ||
| // pair of Oak Channels. | ||
| service RoughtimeService { | ||
| rpc GetRoughTime(RoughTimeRequest) returns (RoughTimeResponse); | ||
| rpc GetRoughtime(RoughtimeRequest) returns (RoughtimeResponse); |
There was a problem hiding this comment.
We should probably stick to standard naming conventions if possible: https://cloud.google.com/apis/design/naming_convention#method_names
| let timeout_seconds = if config.timeout_seconds == 0 { | ||
| DEFAULT_TIMEOUT_SECONDS | ||
| } else { | ||
| config.timeout_seconds as u64 | ||
| }; |
There was a problem hiding this comment.
If these values are meant to be optional, then perhaps should use proper types in the protos, e.g. https://developers.google.com/protocol-buffers/docs/reference/csharp/class/google/protobuf/well-known-types/u-int32-value, which prost should convert into Option<u32> , which you can then use as config.server_retries.unwrap_or(DEFAULT_SERVER_RETRIES) etc.
| fn process_invocation(&self, runtime: &RuntimeProxy, invocation: &Invocation) { | ||
| match invocation.receive_request(runtime) { | ||
| Ok(request) => { | ||
| if request.method_name != "/oak.roughtime.RoughtimeService/GetRoughtime" { |
There was a problem hiding this comment.
Should this be auto generated at some point? Do we have a GitHub issue to track that already?
There was a problem hiding this comment.
Created #1113
conradgrobler
force-pushed
the
rt_node
branch
3 times, most recently
from
7c79dd7 to
a914ee1
Compare
oak/proto/application.proto
Outdated
| @@ -119,10 +121,10 @@ message RoughtimeClientConfiguration { | |||
| repeated RoughtimeServer servers = 1; | |||
| // Connection parameters; default values will be used if any parameter is | |||
| // zero. | |||
There was a problem hiding this comment.
| // zero. | |
| // unset. |
Checklist
cover any TODOs and/or unfinished work.
construction.
Fixes #730