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

Log parsing errors using log pkg. #829

Closed
wants to merge 654 commits into from

Conversation

dilyevsky
Copy link
Contributor

@dilyevsky dilyevsky commented Apr 2, 2019

Gets rid of annoying stderr messages. Consistent with the rest of the file.


This change is Reviewable

iamqizhao and others added 30 commits April 19, 2016 09:40
…ion.

See grpc/grpc-go#642 for the corresponding grpc-go change.

Signed-off-by: David Symonds <dsymonds@golang.org>
Signed-off-by: David Symonds <dsymonds@golang.org>
Unmarshaling Any is a TODO.

Fixes golang#170.
Methods that manipulate protos with extensions will now take
proto.Message instead of the internal extendableProto interface.

A ClearExtensions method is added to clear all extensions on protos.
While this check could be done before marshalling, it shouldn't occur
frequently and thus would slow down proto encoding on average.
Turn generated message struct field XXX_Extensions map[int32]Extension
into an embedded proto.InternalExtensions  struct

InternalExtensions is a struct without any exported fields and methods.
This effectively makes the representation of the extension map private.
The proto package can access InternalExtensions by checking that the
generated struct has the method 'extmap() proto.InternalExtensions'.

Also lock accesses to the extension map.

This change bumps the Go protobuf generated code version number. Any
.pb.go files generated with this version of the proto package or later
will require this version or later of the proto package to compile.
When the new V2 generated extension format was introduced, we
mistakenly dropped support for comparing V1 generated extensions
for equality. Add that back.
This is meant to fix golang#188. There
are no tests because we don't guarantee that we're going to maintain
this behavior in the future.

Fixes golang#188
Otherwise invalid JSON can be produced.
Bump support package version number from 2 to 3.
Add tests for merging a map field. When src contains a duplicate key,
its value message replaces (not merges with) the value message in dst.
// GetAllExtensionDescs returns a slice of all the descriptors
// extensions present in pb.
// // If an extension is not registered, a descriptor with only the
// 'field' value set will
// // be returned instead of a full descriptor.
// // The returned slice is not guaranteed to be in any given order.
// func GetAllExtensionDescs(pb Message) (extensions []*ExtensionDesc,
// err error)
A test depends on the new 'Cute' field.
I added a new Unmarshaler type for symmetry with Marshaler.
Change-Id: Icb29ca6bfa96fbc071cebaac4fc4f5dcbfd1d252
When marshalling to JSON, sort numeric proto keys in numeric order per
https://developers.google.com/protocol-buffers/docs/proto#maps.

Change-Id: Iab5bdfbf599ce35e856ad4d0e503fd2ab1a9aacd
Change-Id: Icf8b82a07500ab226cf24b4c6aacc9a0a7476a59
This avoids people specifying protos (inadvertently) as:

some_repeated_any {
  [type.googleapis.com/a/path/proto.Type] {
    blah: 1
  }
  [type.googleapis.com/a/path/proto.Type] {
    blah: 2
  }
}

when they meant:

some_repeated_any {
  [type.googleapis.com/a/path/proto.Type] {
    blah: 1
  }
}
some_repeated_any {
  [type.googleapis.com/a/path/proto.Type] {
    blah: 2
  }
}
dsnet and others added 12 commits November 19, 2018 16:18
Update all proto files that is obtained from the upstream repository to v3.6.1.
…olang#760)

The marshaler, unmarshaler, and sizer functions are unused ever since
the underlying implementation was switched to be table-driven.
Change the function to only return the wrapper structs.

This change:
* enables generated protos to drop dependencies on certain proto types
* reduces the size of generated protos
* simplifies the implementation of oneofs in protoc-gen-go

Updates golang#708
The generator currently uses an unintuitive and stateful algorithm
for name generation where it "fixes" name conflicts by appending "_"
to the end of the new name.

PR#657 refactored the generator code and noticed that the above
algorithm was not properly taking into account that a Get method is
generated for parent oneofs, fixing it in the same PR. While this is
more correct, this breaks users (see golang#780) since it means that the
generation of names can change.

This PR changes the name mangling logic to be as it was previously.
This does mean that some new proto files may be unbuildable,
but that is arguably better than breaking existing proto files.

Fixes golang#780
It is spec'd that way, but best practice is to avoid it.
The existing code is much more complex.
Removes two go mod dependencies on golang.org/x/sync and 
indirectly on golang.org/x/net. These were being used to provide 
an errgroup.Group to collate errors in a test.

This PR provides the same functionality but using sync.WaitGroup.
The conformance test is hard to run and heavily dependant on the exact
version of protoc installed on the developer's workstation.
Delete this from the v1 and leave conformance testing to v2,
which has infrastructure built for stricter control over dependencies.
This changes protoc-gen-go-grpc to always emit an UnimplementedServer type
that implements the service interface. Users trying to implement
the service interface can embed this type to ensure that new methods
can be added in a forwards compatible manner.
Avoid using an errUnimplemented function, which requires us to
give a name to it, causing possible conflicts.
Instead, just inline it's functionality.
…ng#820)

In the rare event that a proto file only has a service declaration with
no declared methods, it will not depend on status and code.
Make sure these are not imported in such cases.
@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

1 similar comment
@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

1 similar comment
@googlebot
Copy link
Collaborator

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@dsnet
Copy link
Member

dsnet commented Apr 2, 2019

This API is only intended for use with generated messages from protoc-gen-go, which does not generate messages that triggers this. How were you able to trigger these failures in the first place?

@dilyevsky
Copy link
Contributor Author

This comes up a lot when trying to render Kubernetes psuedo-protos. Specifically this: https://github.com/kubernetes/apimachinery/blob/7b3d41122501114701f8d013ea1d9533f60c5dd5/pkg/apis/meta/v1/time.go#L34

@gopherbot gopherbot closed this Apr 16, 2019
@dilyevsky dilyevsky deleted the fix/no-log-stderr branch April 16, 2019 15:58
@dilyevsky dilyevsky restored the fix/no-log-stderr branch April 16, 2019 15:58
@dilyevsky
Copy link
Contributor Author

Looks like history got overwritten. @dsnet should i re-open this or send it to some other repo?

@neild
Copy link
Contributor

neild commented May 15, 2019

We don't support message types other than the ones produced by protoc-gen-go, and I don't think we should set a precedent of accepting changes to support those types.

@dilyevsky
Copy link
Contributor Author

dilyevsky commented May 15, 2019 via email

@neild
Copy link
Contributor

neild commented May 15, 2019

That's fair, and on a second look I do see that there are a lot of log.Print calls. We should be consistent. Go ahead and re-open this. (It was accidentally closed due to a config error--sorry!)

FWIW, the in-development revision of the package API neither logs nor prints.

@dilyevsky
Copy link
Contributor Author

Thanks for taking a look, @neild. Re-opened as #851.

@golang golang locked and limited conversation to collaborators Jun 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet