-
-
Notifications
You must be signed in to change notification settings - Fork 91
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
Add option for configurable post-start hooks #1699
Conversation
8ff8c9c
to
8014af3
Compare
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 think "hook" is too generic. We can be more specific here: it's really just a sequence of commands and we can call them what they are.
The new option `vast.start.hooks` allows for specifying an ordered list of VAST commands that run inside the node after successful startup. This is useful for declarative deployments that run sources or matchers (via the proprietary Live Matching plugin) in the node.
8014af3
to
9f7332d
Compare
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 seems like a good middleground to get experience with post-hoc configuration of VAST using commands with side effects.
That said, I cringe a bit in that it could be more declarative to fit the surrounding configuration. I don't think that's a dealbreaker though; this is better than the current state.
This works around the static assertion failure "passing views as lvalues is disallowed" that occured only with the pinned {fmt} revision in CI.
I received @lava's approval to go ahead with this design in a call earlier. |
📔 Description
The new option
vast.start.commands
allows for specifying an ordered list of VAST commands that run inside the node after successful startup. This is useful for declarative deployments that run sources or matchers (via the proprietary Live Matching plugin) in the node.📝 Checklist
🎯 Review Instructions
Try locally. This can hardly be tested, sadly, since we do not have unit tests for the start command and cannot reliably test remote sources in the integration test framework. I played around with the following: