turbo
turbo copied to clipboard
Fixes #1993 and #2096 adds a client timeout and additional logging
This fixes #1993 and 2 parts of #2096 by adding a client timeout config option and additionally logging any status that is not http.StatusOK.
This is my first attempt at go, FYI. So I'll take any feedback you can give me (and I'm also going to have my coworkers who work in Go daily give me some feedback too)
All tests passed and so did the e2e tests
cc: @mehulkar
@tm1000 is attempting to deploy a commit to the Vercel Team on Vercel.
A member of the Team first needs to authorize it.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated |
|---|---|---|---|---|
| examples-designsystem-docs | 🔄 Building (Inspect) | Visit Preview | Nov 17, 2022 at 5:27PM (UTC) | |
| turbo-site | ✅ Ready (Inspect) | Visit Preview | 💬 Add your feedback | Nov 17, 2022 at 5:27PM (UTC) |
5 Ignored Deployments
| Name | Status | Preview | Comments | Updated |
|---|---|---|---|---|
| examples-basic-web | ⬜️ Ignored (Inspect) | Visit Preview | Nov 17, 2022 at 5:27PM (UTC) | |
| examples-kitchensink-blog | ⬜️ Ignored (Inspect) | Visit Preview | Nov 17, 2022 at 5:27PM (UTC) | |
| examples-native-web | ⬜️ Ignored (Inspect) | Visit Preview | Nov 17, 2022 at 5:27PM (UTC) | |
| examples-nonmonorepo | ⬜️ Ignored (Inspect) | Visit Preview | Nov 17, 2022 at 5:27PM (UTC) | |
| examples-svelte-web | ⬜️ Ignored (Inspect) | Visit Preview | Nov 17, 2022 at 5:27PM (UTC) |
Excellent first-pass at Go!
It's hard to balance how we should configure this with the reality of trying to migrate our cache syncing layer to the daemon. Below you'll see the chart I made while trying to rationalize what our behavior here is.
We're in a transition between "one-off" and "long-running" configuration as some of these things will be transitioned to the daemon. Environment vars are in some kind of dead zone between "one-off" and "permanent" ("permanent" on CI, weirdly mutable on local boxes so possibly one-off), and flags really only make sense in the one-off situation.
In a future story this is a daemon configuration in the generalized case, not a per-run configuration (though no-daemon's existence necessitates per-run configuration as well). And, timeout in particular isn't really a repository-level configuration, it's a user level configuration, or even (user + repository + remote cache) level configuration. (Your distance to the cache and speed of your box isn't a shared property.)
So, to get this landed:
- Two naming things:
--remote-cache-timeoutandTURBO_REMOTE_CACHE_TIMEOUT. (Existing vars will be renamed to that strategy in some future change.) - We need tests to ensure that we prevent setting this property via the repository config. (I don't really care if the solution for preventing it is elegant, I just want to avoid creating an API we have to deprecate.)
| Flags | Env | Config File | Go: eventually read from where | Comments |
|---|---|---|---|---|
| --no-daemon | run.runOpts.noDaemon | Output monitoring can cause turbo to skip restoration from a cache hit. This is related but not explicitly connected. | ||
| --force | TURBO_FORCE | runcache.Opts.SkipReads → runcache.RunCache.readsDisabled | The Go names are far more expressive as to what actually happens. | |
| --no-cache | runcache.Opts.SkipWrites → runcache.RunCache.writesDisabled | The Go names are far more expressive as to what actually happens. | ||
| --cache-dir | cache.Opts.OverrideDir → cache.fsCache.cacheDirectory | Either an absolute path, or a path relative to the repository root. Default: node_modules/.cache/turbo/ | ||
| --remote-only | TURBO_REMOTE_ONLY | cache.Opts.SkipFilesystem | Disables reading and writing from the filesystem cache. Does not guarantee that the remote cache is actually enabled. Turborepo still creates cache outputs, but possibly sends them to the no-op cache if remote caching isn’t available. This exists as a workaround for unbounded growth of the cache in CI which may do their own caching from the filesystem. | |
| cache.Opts.SkipRemote | Computed: user.token && (repo.teamid OR repo.teamslug) After those values are resolved from flags and env. |
|||
| --cache-workers | cache.Opts.Workers | 10 by default. Makes cache.Put async. When async this enables turbo to proceed to the next build task prior to moving all files into the cache. | ||
| --preflight | client.ApiClient.usePreflight | These nine items are all “how do I talk to the server?” They’re inconsistently configurable. | ||
| --token | TURBO_TOKEN VERCEL_ARTIFACTS_TOKEN | user.token | userConfig.Token() → repoConfig.GetRemoteConfig() -> client.remoteConfig -> client.ApiClient.token | |
| TURBO_TEAMID VERCEL_ARTIFACTS_OWNER | repo.teamid | repoConfig.GetRemoteConfig() -> client.remoteConfig -> client.ApiClient.teamID | TURBO_TEAMID was incidentally created during the Viper adoption; it is entirely undocumented. | |
| --team | TURBO_TEAM | repo.teamslug | repoConfig.GetRemoteConfig() -> client.remoteConfig -> client.ApiClient.teamSlug | |
| --api | TURBO_API | repo.apiurl | repoConfig.GetRemoteConfig() -> client.remoteConfig -> client.ApiClient.baseUrl | |
| --login | TURBO_LOGIN | repo.loginurl | config.RepoConfig.LoginUrl() | |
| turbo.remoteCache.signature | cache.Opts.RemoteCacheOpts.Signature | This is 1:1 from the turbo.json parse. | ||
| turbo.remoteCache.teamId | cache.Opts.RemoteCacheOpts.TeamID | ❌ Parsed but unused. Not exposed in the schema. | ||
| TURBO_REMOTE_CACHE_SIGNATURE_KEY | os.Getenv | This is only grabbed at the moment of validation. |
@nathanhammond thanks!
Which file is the repository Config again? Just trying to keep all the Configs in mind. I think you're just saying don't allow the setting in the turbo.json Config file. Only allow it as a cli arg or environment variable
@tm1000
https://github.com/vercel/turbo/blob/6a0228125cafbbceed53ff17a7b25259422df568/cli/internal/config/config_file.go#L138
https://github.com/vercel/turbo/blob/6a0228125cafbbceed53ff17a7b25259422df568/cli/internal/config/config_file.go#L180-L182
So, we need to make sure that it is not read in from <repo-root>/.turbo/config.json.
@nathanhammond changes applied as requested.
@nathanhammond @chris-olszewski @mehulkar hey team!
Any update on this? I know you're all extremely busy with the amazing things at Vercel but I hate having to build my own turborepo binary and distribute it with my packages. I'd love for this to be merged/included.
@tm1000 we're tracking this to land before the 1.7 release, but don't have a strong timeline on when that will be! In other words, even if we got this PR merged today, we don't have a strong sense of when the next release would be.
@mehulkar hey that works for me! Good to know at least 1.7 :-D
@chris-olszewski completed as requested
@chris-olszewski so it's failing now due to massive changes that happened in config_file.go
@tm1000 I'll deal with the changes coming from #2382. You happen to have caught us in an awkward transition that should get a lot better once #2733 is ready and lands.
Hey @chris-olszewski.
Any updates on this? Im currently bundling my own special build of turborepo to get around client timeouts.
@NicholasLYang I'm going to move to get this merged in, but it's going to have to be ported over to Rust land.
@nathanhammond you're the best. Since it's all rust im going to close this and review the rust port. I <3 rust.