-
Notifications
You must be signed in to change notification settings - Fork 911
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
Order specifications for hooks #4168
Changes from 3 commits
11a0e38
f86b66b
b5cab17
05fe058
c3afd29
3101bef
5124378
2ca13b2
77c27af
0ed8a0c
ed8f556
7d41997
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -117,7 +117,6 @@ static void destroy_plugin(struct plugin *p) | |
{ | ||
struct plugin_rpccall *call; | ||
|
||
plugin_hook_unregister_all(p); | ||
list_del(&p->list); | ||
|
||
/* Terminate all pending RPC calls with an error. */ | ||
|
@@ -1057,20 +1056,49 @@ static const char *plugin_subscriptions_add(struct plugin *plugin, | |
static const char *plugin_hooks_add(struct plugin *plugin, const char *buffer, | ||
const jsmntok_t *resulttok) | ||
{ | ||
const jsmntok_t *hookstok = json_get_member(buffer, resulttok, "hooks"); | ||
const jsmntok_t *t, *hookstok, *beforetok, *aftertok; | ||
size_t i; | ||
|
||
hookstok = json_get_member(buffer, resulttok, "hooks"); | ||
if (!hookstok) | ||
return NULL; | ||
|
||
for (int i = 0; i < hookstok->size; i++) { | ||
char *name = json_strdup(tmpctx, plugin->buffer, | ||
json_get_arr(hookstok, i)); | ||
if (!plugin_hook_register(plugin, name)) { | ||
json_for_each_arr(i, t, hookstok) { | ||
char *name, *depfail; | ||
struct plugin_hook *hook; | ||
|
||
if (t->type == JSMN_OBJECT) { | ||
const jsmntok_t *nametok; | ||
|
||
nametok = json_get_member(buffer, t, "name"); | ||
if (!nametok) | ||
return tal_fmt(plugin, "no name in hook obj %.*s", | ||
json_tok_full_len(t), | ||
json_tok_full(buffer, t)); | ||
name = json_strdup(tmpctx, buffer, nametok); | ||
beforetok = json_get_member(buffer, t, "before"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it took me a second to fully grasp the implication of 'before' and 'after'. Renaming them to "run_before" and "run_after" might be more immediately obvious? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's more obvious in context, than when reading in a patch, e.g.: "hooks": [
{"name": "htlc_accepted",
"before": ["someplugin.py"],
"after": ["foo.py"]}
] |
||
aftertok = json_get_member(buffer, t, "after"); | ||
} else { | ||
name = json_strdup(tmpctx, plugin->buffer, t); | ||
beforetok = aftertok = NULL; | ||
} | ||
|
||
hook = plugin_hook_register(plugin, name); | ||
if (!hook) { | ||
return tal_fmt(plugin, | ||
"could not register hook '%s', either the " | ||
"name doesn't exist or another plugin " | ||
"already registered it.", | ||
name); | ||
} | ||
|
||
plugin_hook_add_deps(hook, plugin, buffer, beforetok, aftertok); | ||
depfail = plugin_hook_make_ordered(tmpctx, hook); | ||
if (depfail) | ||
return tal_fmt(plugin, | ||
"Cannot correctly order hook %s:" | ||
"conflicts in %s", | ||
name, depfail); | ||
tal_free(name); | ||
} | ||
return NULL; | ||
|
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.
General question about 'plugin names': Is this same as for
cli plugin start/stop
commands, meaning either full path (useless for this feature) or basename? If so, we may note that the plugin name includes its extension, meaning full filename.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, filename or full path name.