Avoid reinstalling env templates if Git source has not changed - #3676
Open
itowlson wants to merge 1 commit into
Conversation
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3672.
The adopted fix is to record against each template the repo and tag that it was installed from, assume that tags and branches are immutable, and update only if the actual tag or branch listed in the env file has changed from the one used for installation.
But: two considerations:
This will result in missed updates if the env owner moves a tag or branch in their templates repo. For this to work, env owners have to consider refs immutable, and if they want to change their templates, they have to update their env with a new tag. I am not sure if this is a safe assumption. We could have a Plan B to re-fetch periodically, but either this kick in too often (and Thorsten would remain frustrated), or it would be too infrequent to be useful. Feedback welcome.
This does not address the delay to re-snapshot the environment, which happens if the environment is unversioned and our snapshot is more than 1 hour old (basically if no unversioned env has been used in the last hour). This is trickier because we can have absolutely no way to tell if the upstream has changed, without a network round-trip. We could increase the recency window to reduce the incidence of these delays, but again that just makes it more and more likely that a user will hit the window. Although we do have the
spin targets updatecommand to mitigate that, so maybe it would be okay.So: feedback and discussion first, so we don't do the wrong thing; but at the same time, it would be nice to get any fix we are happy with into 4.1. So no mixed messages then.
(cc @ThorstenHans who flagged the original issue)