-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
type_url set correctly from container structure. #1091
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1091 +/- ##
=======================================
Coverage 54.51% 54.51%
=======================================
Files 16 16
Lines 1229 1229
=======================================
Hits 670 670
Misses 386 386
Partials 173 173 Continue to review full report at Codecov.
|
services/containers/helpers.go
Outdated
@@ -12,6 +12,11 @@ import ( | |||
"google.golang.org/grpc/codes" | |||
) | |||
|
|||
const ( | |||
//URLTypePrefix adds prefix to make OCI spec version as per protobuf's TypeUrl | |||
URLTypePrefix = "types.containerd.io/opencontainers/runtime-spec/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: TypeUrlPrefix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be io.containerd.types/
?
the event package is using `types.containerd.io` atm (cc @stevvoe, I
believe that's what google documentation recommends, as it needs to be an
URL you can navigate to from a web-browser)
On Mon, Jun 26, 2017 at 2:01 PM Michael Crosby ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In services/containers/helpers.go
<#1091 (comment)>
:
> @@ -12,6 +12,11 @@ import (
"google.golang.org/grpc/codes"
)
+const (
+ //URLTypePrefix adds prefix to make OCI spec version as per protobuf's TypeUrl
+ URLTypePrefix = "types.containerd.io/opencontainers/runtime-spec/"
Wouldn't it be io.containerd.types/?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1091 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/APUifkUyr_qb0I8VxAk50r14jAViVwC3ks5sIBwKgaJpZM4OE5Ic>
.
--
Sent from my phone.
|
c2f9870
to
3b7d1eb
Compare
Updated with suggested changes. //cc @mlaventure |
Fixes containerd#1052 Signed-off-by: Kunal Kushwaha <kushwaha_kunal_v7@lab.ntt.co.jp>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/cc @stevvooe
Does #1133 more completely handle this issue? |
Fixes #1052
This fix, sets the
type_url
correctly, while reading information fromcontainer
, without storing changes incontainer
struct.Signed-off-by: Kunal Kushwaha kushwaha_kunal_v7@lab.ntt.co.jp