spin new -E: update unversioned environments if not recent#3523
Open
itowlson wants to merge 1 commit into
Open
spin new -E: update unversioned environments if not recent#3523itowlson wants to merge 1 commit into
spin new -E: update unversioned environments if not recent#3523itowlson wants to merge 1 commit into
Conversation
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
Contributor
kate-goldenring
left a comment
kate-goldenring
left a comment
There was a problem hiding this comment.
just the unwrap handling and then LGTM
| } | ||
| } | ||
|
|
||
| fn is_unversioned(id: &str) -> bool { |
Contributor
There was a problem hiding this comment.
We should update the readme of spin-environments to specify that versioning is not needed.
| let git_source = GitSource::new(&url, None, &self.git_root); | ||
| if self.git_root.exists() { | ||
| git_source.pull().await | ||
| git_source.pull().await.unwrap(); |
Contributor
There was a problem hiding this comment.
Why are we unwrapping here? This could panic on a network error
Suggested change
| git_source.pull().await.unwrap(); | |
| git_source.pull().await?; |
| } | ||
|
|
||
| /// Updates if we have not updated recently, ignoring | ||
| /// failures. The scenario here is unverioned environments, |
Contributor
There was a problem hiding this comment.
nit
Suggested change
| /// failures. The scenario here is unverioned environments, | |
| /// failures. The scenario here is unversioned environments, |
| } | ||
| } | ||
|
|
||
| fn last_update_file(&self) -> Result<PathBuf, ()> { |
Collaborator
There was a problem hiding this comment.
Would -> Option<PathBuf> be more idiomatic here?
fibonacci1729
approved these changes
May 21, 2026
Collaborator
fibonacci1729
left a comment
fibonacci1729
left a comment
There was a problem hiding this comment.
LGTM pending Kate's unwrap() and typos.
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
3 participants
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.
The
spin new -Efeature assumed that environments were immutable - that a new environment could get a new version. This is true for self-host platforms such as the CLI and SpinKube. However, for hosted environments it leads to a weird UI. Let us return to Bob's Discount Cloud, the premier bargain-basement Spin host. At any give time, BDC supports a specific environment. But the current thing requires users to pick a version. At first Bob tells them to writespin new -E bdc@1, which is a bit weird but uh okay; but then later Bob must raze all those docs from the Internet and get them to writespin new -E bdc@2, instead. Really Bob wants users to be able to writespin up -E bdcand get whatever is current in his Discount Cloud.We could tackle this by forcing an update of the environments repo on every
get. However, this introduces a delay. If you're on airplane wifi, that could be quite a loooooooooong delay.This PR proposes to track the recency of the catalogue snapshot, and to update it only if 1. the env being referenced is unversioned; and 2. the snapshot is older than 1 hour. In addition, such updates will be timed out after 2 seconds to avoid slow networks making them painful.
This hopefully minimises user annoyance; and hopefully Bob's updates will not usually be time-sensitive to the hour (if they are, Bob will have to ask users to purge their envs cache, or to
spin new -E some-fake-envto force a fetch: we can look at adding commands for this if need be).(And... I realised while working on this that
spin newcurrently always does a git fetch anyway, to force-install templates from the referenced repo. So maybe this was a pointless optimisation. But I'd like to fix that soon, so I'm not going to bother backing it out...)Tested by pointing a build at a fork of the environments repo, dialling the recency timeout down to a few minutes, and logging when pulls did and didn't happen (as well as verifying that changes were reflected in the
spin newUI).