-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
fix: Generate useful error message when no expression on hook #10919
Conversation
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.
Is this ready for review?
Not yet, the validator isn't quite done. |
7de2e80
to
cb27c36
Compare
The error message for a missing or empty expression was `unable to evaluate expression '': unexpected token EOF (1:1)`. This changes it to a more useful `Expression required for hook <hook path>`. Also attempt to validate to prevent this problem upfront. Signed-off-by: Alan Clucas <alan@clucas.org>
cb27c36
to
837ce52
Compare
@@ -18,12 +19,15 @@ func (woc *wfOperationCtx) executeWfLifeCycleHook(ctx context.Context, tmplCtx * | |||
if hookName == wfv1.ExitLifecycleEvent { | |||
continue | |||
} | |||
hookNodeName := generateLifeHookNodeName(woc.wf.ObjectMeta.Name, string(hookName)) |
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 is not needed until inside the execute block?
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 use it on new line 24, two lines below this one as part of the error message.
Signed-off-by: Alan Clucas <alan@clucas.org>
…oj#10919) Signed-off-by: Alan Clucas <alan@clucas.org>
…oj#10919) Signed-off-by: Alan Clucas <alan@clucas.org> Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
The error message for a missing or empty expression was
unable to evaluate expression '': unexpected token EOF (1:1)
. This changes it to a more usefulExpression required for hook <hook path>
.Also attempt to validate to prevent this problem upfront.
Fixes #10910