-
Notifications
You must be signed in to change notification settings - Fork 74
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
(TK-487) Allow friendly init/start fail fast via ::exit throw #289
base: main
Are you sure you want to change the base?
Conversation
This isn't quite ready (e.g. at least needs tests), but I wanted to see if it seemed plausible before proceeding further. The current motivation is wanting to be able to shut down in a friendly way when a service detects a problem with the configuration it retrieves via |
CLA signed by all contributors. |
|
c450c97
to
3ae4471
Compare
3ae4471
to
1a7f88e
Compare
|
||
(defn exit-exception? [ex] | ||
(and (instance? ExceptionInfo ex) | ||
(not (schema/check {(schema/optional-key :puppetlabs.trapperkeeper.core/exit) |
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.
I'm a confused why there is a not
here..
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.
Oh, right, because I believe check
returns nil
if it matches, info about the mismatch, otherwise.
[exception] | ||
(if (exit-exception? exception) | ||
(merge {:cause :requested} | ||
(select-keys (ex-data exception) [:puppetlabs.trapperkeeper.core/exit])) |
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.
Why select-keys
instead of including all the exception data?
Also, based on your documentation, it looks like :puppetlabs.trapperkeeper.core/exit
is not a top-level key, it's the value for the :kind
key. so not sure what the intention was here.
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.
If I recall correctly, it's just to be sure that the exception only contributes the key that it's supposed to, e.g. can't clobber other top-level keys, etc.
And I think the idea is that :kind
is a non-namespaced key that tells you what type of ex-data map you have. Everything else in the map can be type-specific. The ':kind` value is namespaced to avoid collisions across namespaces and even across different projects.
We could namespace :kind
, as say :puppetlabs/kind
, but I believe there's some precedence at puppet (and I vaguely recall maybe elsewhere) to use :kind
globally. TK itself is already using :kind
for some exit-related code.
1a7f88e
to
715d414
Compare
Allow init and start methods to throw a request-shutdown style ex-info map to short circuit the startup process and exit with a specified message and status, rather than a backtrace. This just provides a short circuiting (immediate) counterpart to the existing, deferred shutdown requests provided by request-shutdown.
715d414
to
8d56968
Compare
Allow init and start methods to throw a request-shutdown style ex-info
map to short circuit the startup process and exit with a specified
message and status, rather than a backtrace.
This just provides a short circuiting (immediate) counterpart to the
existing, deferred shutdown requests provided by request-shutdown.