add typeurl package for TypeUrl handling for external types #1133
Conversation
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
| @@ -0,0 +1,99 @@ | |||
| package typeurl | |||
There was a problem hiding this comment.
This can just be a move of events/convert.go.
typeurl/types.go
Outdated
| "github.com/gogo/protobuf/types" | ||
| ) | ||
|
|
||
| const Namespace = "types.containerd.io" |
There was a problem hiding this comment.
Prefix? Such that it reads a typeurl.Prefix.
typeurl/types.go
Outdated
| } | ||
|
|
||
| // TypeUrl returns the type url for a registred type | ||
| func TypeUrl(v interface{}) (string, error) { |
There was a problem hiding this comment.
types.Any.TypeUrl would disagree ;)
typeurl/types.go
Outdated
| defer mu.Unlock() | ||
| u, ok := registry[tryDereference(v)] | ||
| if !ok { | ||
| return "", ErrNotExists |
There was a problem hiding this comment.
Fallback to checking protobuf registration.
There was a problem hiding this comment.
See events/convert.go and proto.MessageName(pb). I think you can dispatch on proto.Message.
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #1133 +/- ##
==========================================
- Coverage 34.26% 31.69% -2.57%
==========================================
Files 28 27 -1
Lines 2335 2174 -161
==========================================
- Hits 800 689 -111
+ Misses 1364 1337 -27
+ Partials 171 148 -23
Continue to review full report at Codecov.
|
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
|
|
||
| var de events.DynamicEvent | ||
| if err := events.UnmarshalEvent(env.Event, &de); err != nil { | ||
| v, err := typeurl.UnmarshalAny(env.Event) |
There was a problem hiding this comment.
I went back and forth on dynamic vs allocated UnmarshalAny and in certain cases, the dynamic approach avoids a serialization if you use the Is function.
task.go
Outdated
|
|
||
| if e.ID == t.containerID && e.Pid == t.pid { | ||
| return e.ExitStatus, nil | ||
| v, err := typeurl.UnmarshalAny(evt.Event) |
There was a problem hiding this comment.
Using Is here prevent unmarshaling from happening.
typeurl/types.go
Outdated
| mu sync.Mutex | ||
| registry = make(map[reflect.Type]string) | ||
| ErrRegistered = errors.New("typeurl: type already registred") | ||
| ErrNotExists = errors.New("typeurl: type is not registered") |
There was a problem hiding this comment.
Use errdefs.ErrAlreadyExists and errdefs.ErrNotFound.
|
LGTM after a few nits. |
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
| if err != nil { | ||
| return nil, err | ||
| } | ||
| v := reflect.New(t.t).Interface() |
There was a problem hiding this comment.
Inside of EmptyAny, they always do reflect.New(t.t.Elem()).Interface(), which indicates they always work with pointer types for registration. We can enforce a pointer type at registration to prevent the wrong type from being registered.
There was a problem hiding this comment.
updated to panic when you try to get the type of something that is a non-pointer
|
LGTM |
typeurl/types_test.go
Outdated
| registry = make(map[reflect.Type]string) | ||
| } | ||
|
|
||
| func TestRegsiterPointerGetPointer(t *testing.T) { |
There was a problem hiding this comment.
very minor nit: TestRegisterPointerGetPointer (vs. RegsiterPointer..)
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
|
@estesp fixed the typo |
|
LGTM |
Use wait instead of `TaskExit`.
This adds a
typeurlpackage for the marshal/unmarshal of Any types with properTypeUrlfields set for the data that they contain.Closes #1091
Fixes #1052