-
Notifications
You must be signed in to change notification settings - Fork 642
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
feat: add Status and ID as event attributes #3232
Conversation
pkg/cmd/system/events.go
Outdated
log.G(ctx).WithError(err).Warn("cannot marshal Any into JSON") | ||
} | ||
|
||
id := "unknown" |
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.
should we initialize as "" which is default for golang
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.
because i think the clients might expect something like null to verify if the ID exist or not
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.
Got it. Changed the default value to be "".
6c2b20b
to
9ea709e
Compare
@AkihiroSuda PTAL when you get the chance |
pkg/cmd/system/events.go
Outdated
} | ||
|
||
id := "" | ||
_, ok := data["container_id"] |
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.
This should be skipped when json.Unmarshal
returned an error?
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.
Yes, it should. I added an else statement, so now this will only run if json.Unmarshal
doesn't return an error.
265bfbd
to
7456760
Compare
Signed-off-by: CodeChanning <[email protected]>
7456760
to
fea8219
Compare
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.
Thanks
Adding Status and ID as attributes
Makes
nerdctl events --format=json
output more docker compatible by adding some missing attributes in nerdctlExample
docker events --format json
output:(src: docker events doc)
Example
nerdctl events --format json
output after change:(includes both status and id in output)