-
Notifications
You must be signed in to change notification settings - Fork 796
node: Log version on watcher start #4472
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
base: main
Are you sure you want to change the base?
node: Log version on watcher start #4472
Conversation
c049d5f
to
ad548f4
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.
Thanks! This is great
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.
The changes seem sensible but I did not run tests.
ad548f4
to
99952a2
Compare
@djb15 is it maybe worth emitting a gauge or something as well in where one of the tags is the version? |
Thanks to @SEJeff for #2520 from which this is based.
This has been tested manually for every watcher in Tilt devnet to confirm we get the expected output.
I have also deliberately induced a panic in a
logVersion
method and confirmed that CI fails. For watchers without tests, the watcher will boot loop in Tilt and eventually timeout. The new changes are designed in such a way that if the version can't be fetched successfully an error is logged and the watcher continues as normal.