Added editions 2023 support with hybrid protobuf api #206
Conversation
|
Here's the code health analysis summary for commits Analysis Summary
|
|
benchmark/qspec.go line 61 is wrong. |
There was a problem hiding this comment.
Very nicely done! Please fix the minor comments, and we can merge this.
One question I have is whether or not to update some or all the .proto files in tests and internal/testprotos, etc., to use editions. We can do it in a separate PR if this makes sense to do.
examples/storage/client.go
Outdated
| @@ -16,7 +16,7 @@ func runClient(addresses []string) { | |||
| } | |||
|
|
|||
| // init gorums manager | |||
| mgr := proto.NewManager( | |||
| mgr := pb.NewManager( | |||
| gorums.WithDialTimeout(1*time.Second), | |||
There was a problem hiding this comment.
I think we removed this WithDialTimeout option in the previous PR.
| @@ -29,13 +30,17 @@ $(static_file): $(static_files) | |||
| @protoc-gen-gorums --bundle=$(static_file) | |||
|
|
|||
| %.pb.go : %.proto | |||
| @protoc -I=$(proto_path) --go_out=paths=source_relative:. $^ | |||
| @protoc -I=$(proto_path) \ | |||
| --go_opt=default_api_level=API_HYBRID \ | |||
There was a problem hiding this comment.
Nit: looks like an extra tab character on this line.
benchmark/qspec.go
Outdated
| resp.TotalOps /= uint64(len(replies)) | ||
| resp.TotalTime /= int64(len(replies)) | ||
| resp.Throughput /= float64(len(replies)) | ||
| resp.SetLatencyVar(resp.GetLatencyVar()/float64(resp.GetTotalOps()) - float64(len(replies))) |
There was a problem hiding this comment.
Referring to your comment related to this line. I think it should be like this:
resp.SetLatencyVar(resp.GetLatencyVar() / (float64(resp.GetTotalOps()) - float64(len(replies))))
|
We would also need to merge master into this and resolve the merge conflicts, and we should fix the DeepSource issues as well. |
added editions support and updated storage example to edition2023
Editions 2024 is still experimental, protoc has support but it is unclear if protoc-gen-go has support for it.
moving hybrid protobuf api
The hybrid API allows for use of both the open and opaque protobuf api methods, and struct fields.
** Changes **
This may not have any effect for the users of gorums, i consider testing the opaque api on relab/raft.
The gorums generated files have not changed.
The example worked both before and after being updated to the opaque API