Skip to content
This repository has been archived by the owner on Aug 14, 2020. It is now read-only.

spec/types: clarify only one eventHandler per type #62

Merged
merged 1 commit into from
Dec 30, 2014

Conversation

jonboulle
Copy link
Contributor

No description provided.

@mpasternacki
Copy link
Contributor

  1. Why use an array here rather than a JSON object? It would seem more natural to me, and it would intuitively ensure only one handler of a given type.
  2. Why explicitly limit event names? Can't an implementation define additional events/handlers? Examples of such handlers would be: ad-hoc maintenance tasks triggered manually (like running chef-server-ctl commands for Chef server); cron-like scheduled tasks; handling situations in multi-application containers when one of the processes exits. Spec may require that if an implementation handles events outside of the spec, the event names should be namespaced (e.g. rocket:cron or jetpack:task) to avoid possible collisions.

@jonboulle
Copy link
Contributor Author

Why use an array here rather than a JSON object? It would seem more natural to me, and it would intuitively ensure only one handler of a given type.

As discussed here and here, this is for consistency with other types, and unfortunately using an object (map) doesn't guarantee us one handler of a given type when it comes to actually unmarshalling from JSON, so I am inclined to stick with the array model.

Why explicitly limit event names? Can't an implementation define additional events/handlers?

We certainly want a small set of very well defined eventHandlers that all executor implementations must support. As for extensions like ad-hoc tasks, is that worth codifying in image manifests at all, rather than just leaving it up to implementations? I think it's a bit of a grey area. Namespacing could be a good compromise and we could define this as a common approach for dealing with other fields like isolators (optional; small set of well-known fields; other fields accepted, but please namespace; commonly-used fields may be promoted to well-known in future versions of the spec)

@jonboulle
Copy link
Contributor Author

I am going to merge this as it reflects the current state of the spec; let's continue the discussion about opening up eventHandler types in #58 /cc @philips

jonboulle added a commit that referenced this pull request Dec 30, 2014
spec/types: clarify only one eventHandler per type
@jonboulle jonboulle merged commit 124153a into appc:master Dec 30, 2014
sttts pushed a commit to sttts/spec that referenced this pull request May 10, 2019
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.

2 participants