Skip to content

Conversation

djb15
Copy link
Collaborator

@djb15 djb15 commented Aug 15, 2025

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.

@djb15 djb15 force-pushed the log-version-on-watcher-start branch 2 times, most recently from c049d5f to ad548f4 Compare August 15, 2025 21:27
@djb15 djb15 marked this pull request as ready for review August 15, 2025 21:39
@djb15 djb15 requested review from evan-gray and panoel as code owners August 15, 2025 21:39
Copy link
Collaborator

@SEJeff SEJeff left a 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

Copy link
Contributor

@johnsaigle johnsaigle left a 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.

@djb15 djb15 force-pushed the log-version-on-watcher-start branch from ad548f4 to 99952a2 Compare September 23, 2025 21:40
@elee1766
Copy link
Contributor

@djb15 is it maybe worth emitting a gauge or something as well in where one of the tags is the version?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants