libimage: add an events system #524
Conversation
|
Podman counter-part -> containers/podman#10219 |
|
LGTM |
|
@containers/podman-maintainers PTAL |
|
Let's wait until containers/podman#10219 turns green. |
|
It feels like it might be better to move the Libpod events code into common and use it directly - I'm concerned by the potentially-competing definitions of Event, for example. |
IMHO that's out of scope of libimage. I try hard to keep libimage self-contained and really don't want to move more code around. |
|
containers/podman#10219 passed the local system tests (remote events is broken), so we could merge. |
|
I wouldn't put the events code in libimage, but elsewhere in c/common. It feels like a good place since it's no longer Podman specific. |
|
I understand but I had enough code consolidation on my shoulders. I need to
stop somewhere. Podman is the only user of events, so I think the design as
is is solid and it gets image events back.
…On Wed 5 May 2021 at 18:55, Matthew Heon ***@***.***> wrote:
I wouldn't put the events code in libimage, but elsewhere in c/common. It
feels like a good place since it's no longer Podman specific.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#524 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACZDRA5VMBQEVFTQNKSZ4P3TMF2ARANCNFSM44EZOBZQ>
.
|
|
@giuseppe @saschagrunert PTAL and get this merged. |
| if r.eventChannel != nil { | ||
| return r.eventChannel | ||
| } | ||
| r.eventChannel = make(chan *Event, 100) |
There was a problem hiding this comment.
Good catch, now it is in Shutdown().
Add an event system to libimage. Callers can opt-in to using events by requesting an event channel via `(*Runtime).EventChannel()`. The returned channel has a buffer of size 100 which should be sufficient even under high loads. But, to be on the safe side, writing an event will time out after 2 seconds to prevent operations from blocking. Currently, the only user of such an event system is Podman which will need to convert the `Event` type to what's used internally in libpod. Signed-off-by: Valentin Rothberg <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
I'm OK with merging as long as we make a card to complete the move of events into common at a later date. |
|
/lgtm |
Regression caused by 011f899. We only want to build actual man pages of course and not general docs. This commit fixes the wildcard target to only use the `.1.md` files instead of all markdown files. Also add the step to the validate target to make sure CI tests this target. Fixes containers#524 Signed-off-by: Paul Holzinger <[email protected]>
Add an event system to libimage. Callers can opt-in to using events by
requesting an event channel via
(*Runtime).EventChannel(). Thereturned channel has a buffer of size 100 which should be sufficient
even under high loads. But, to be on the safe side, writing an event
will time out after 2 seconds to prevent operations from blocking.
Currently, the only user of such an event system is Podman which will
need to convert the
Eventtype to what's used internally in libpod.Signed-off-by: Valentin Rothberg [email protected]