cisco-ie / pipeline-gnmi

A Model-Driven Telemetry collector based on the open-source tool pipeline
Apache License 2.0
32 stars 6 forks source link

jsonpb fix #40

Closed remingtonc closed 5 years ago

remingtonc commented 5 years ago

Resolves #39

=== RUN   TestCodecGPB2JSON
jsonNumber:  {"encoding_path":"","collection_id":1234567890,"collection_start_time":0,"msg_timestamp":0,"data_gpbkv":[],"data_gpb":null,"collection_end_time":0}
jsonString:  {"encoding_path":"","collection_id":"1234567890","collection_start_time":"0","msg_timestamp":"0","data_gpbkv":[],"data_gpb":null,"collection_end_time":"0"}
--- PASS: TestCodecGPB2JSON (0.00s)

Simply completely copies jsonpb.go with the single liner fixes to mitigate failures with Go module usage. Nothing clever, wholesale copy. Updates protobuf dependency in vendor as well.

Removes composites check in go vet due to protobuf dependency...

[pipeline-gnmi] make build                                                                                                                                                                                  jsonpb-fix  ✭ ✱
  >  Building pipeline
go vet .
# github.com/cisco-ie/pipeline-gnmi
./jsonpb.go:847:42: github.com/golang/protobuf/ptypes/struct.Value_NumberValue composite literal uses unkeyed fields
./jsonpb.go:849:42: github.com/golang/protobuf/ptypes/struct.Value_StringValue composite literal uses unkeyed fields
./jsonpb.go:851:42: github.com/golang/protobuf/ptypes/struct.Value_BoolValue composite literal uses unkeyed fields
./jsonpb.go:854:42: github.com/golang/protobuf/ptypes/struct.Value_ListValue composite literal uses unkeyed fields
./jsonpb.go:858:42: github.com/golang/protobuf/ptypes/struct.Value_StructValue composite literal uses unkeyed fields
make: *** [build] Error 2

Apparently go vet is "overly religious" about this issue and it is perfectly functional.

remingtonc commented 5 years ago

Ideally we will remove vendor/ soon and replace with Go modules.

nleiva commented 5 years ago

Making changes to the vendor folder is not a best practice (its content should be automatically populated), but considering we are getting rid of it, this looks good to me.