port(turborepo): Config #4520

NicholasLYang · 2023-04-10T18:35:52Z

Description

Creates an ExecutionState struct that gets serialized to the Go code. The goal is to build up more and more of the info, such as config and graph state in this struct, so that we can do more work on the Rust side.

Testing Instructions

vercel · 2023-04-10T18:36:00Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

9 Ignored Deployments

Name	Status	Updated (UTC)
examples-basic-web	⬜️ Ignored (Inspect)	Apr 25, 2023 7:19pm
examples-cra-web	⬜️ Ignored (Inspect)	Apr 25, 2023 7:19pm
examples-designsystem-docs	⬜️ Ignored (Inspect)	Apr 25, 2023 7:19pm
examples-gatsby-web	⬜️ Ignored (Inspect)	Apr 25, 2023 7:19pm
examples-kitchensink-blog	⬜️ Ignored (Inspect)	Apr 25, 2023 7:19pm
examples-native-web	⬜️ Ignored (Inspect)	Apr 25, 2023 7:19pm
examples-svelte-web	⬜️ Ignored (Inspect)	Apr 25, 2023 7:19pm
examples-tailwind-web	⬜️ Ignored (Inspect)	Apr 25, 2023 7:19pm
examples-vite-web	⬜️ Ignored (Inspect)	Apr 25, 2023 7:19pm

vercel · 2023-04-10T18:36:02Z

@NicholasLYang is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

mehulkar

The code makes sense to me (to the extent that I can pick out the nuances of Rust). I'd be a little wary of merging this without some manual testing though if you haven't already because we have no automated checks for remote cache config and it's a high risk feature.

mehulkar · 2023-04-10T19:57:58Z

cli/internal/client/client.go

-	TeamID   string
-	TeamSlug string
-	APIURL   string
-}


We have no integration testing around this at the moment, so need to be extra careful. I'm hoping to finish #4309 soon (feel free to pick it up for this purpose) so we can make this safer and get more clarity about the expected network requests.

Seconding this

Yeah I'm gonna block this PR on #4309

cli/internal/run/run.go

mehulkar · 2023-04-10T20:01:00Z

crates/turborepo-lib/src/config/repo.rs

-    loginurl: Option<String>,
-    teamslug: Option<String>,
-    teamid: Option<String>,
+    #[serde(rename = "apiurl")]


what are all the renames for?

loginurl is not idiomatic Rust naming, so I figured it's better to have login_url in the Rust code then rename it to loginurl via Serde

that makes sense, but I wasn't sure where the non-idiomatic versions (e.g. loginurl) are used and why we can't replace that?

It's used in the config, and we want to avoid accidentally signing people out/unlinking them when we move from the Go to the Rust version.

crates/turborepo-lib/src/execution_state.rs

crates/turborepo-lib/src/config/user.rs

chris-olszewski · 2023-04-11T17:04:58Z

crates/turborepo-lib/src/config/user.rs

+            .add_source(Environment::with_prefix("TURBO").source(environment.clone()))
+            .add_source(Environment::with_prefix("VERCEL_ARTIFACTS").source(environment))


Not important, but these prefixes all get made lowercase: https://github.com/mehcode/config-rs/blob/master/src/env.rs#L247

yeah I mostly made them uppercase because I tend to see environment variables in uppercase and it was confusing me to see the prefixes as lowercase.

crates/turborepo-lib/src/commands/mod.rs

crates/turborepo-lib/Cargo.toml

chris-olszewski · 2023-04-11T17:30:01Z

cli/internal/cmdutil/cmdutil_test.go

-func TestRemoteCacheTimeoutFlag(t *testing.T) {
-	args := &turbostate.ParsedArgsFromRust{
-		CWD:                "",
-		RemoteCacheTimeout: 599,
-	}
-	h := NewHelper("test-version", args)
-
-	base, err := h.GetCmdBase(args)
-	if err != nil {
-		t.Fatalf("failed to get command base %v", err)
-	}
-
-	assert.Equal(t, base.APIClient.HTTPClient.HTTPClient.Timeout, time.Duration(599)*time.Second)
-}


Do we want to keep one of these unit tests just to make sure that the timeout passed by Rust gets converted to the correct Go duration?

chris-olszewski · 2023-04-11T17:31:55Z

cli/internal/client/client.go

-	TeamID   string
-	TeamSlug string
-	APIURL   string
-}


Seconding this

cli/internal/cmdutil/cmdutil.go

crates/turborepo-lib/src/config/user.rs

turborepo-tests/integration/tests/config.t

mehulkar · 2023-04-21T22:20:20Z

turborepo-tests/integration/tests/config.t

+  }
+
+Run test run with team overloaded from both env and flag (flag should take precedence)
+  $ TURBO_TEAM=vercel ${TURBO} run build --__test-run --team turbo


One more test for both TURBO_TOKEN and --token and VERCEL_ARTIFACTS_TOKEN would be awesome

turborepo-tests/integration/tests/config.t

mehulkar

One question on a test case that looks unexpected, but otherwise looks good!

cli/internal/cmdutil/cmdutil.go

mehulkar · 2023-04-24T20:50:19Z

cli/internal/turbostate/turbostate.go

+	TeamSlug     string `json:"team_slug"`
+	APIURL       string `json:"api_url"`
+	UsePreflight bool   `json:"use_preflight"`
+	Timeout      uint64 `json:"timeout"`


We can do this on the rust side, but it would be good to document where these values can come from (flag, xdg config, ./.turbo/config.json, env var, etc). I always forget that teamId and token are in separate places

Well it's kinda all three, right? Like APIUrl can be overloaded by a flag, by .turbo/config.json and by env var.

yeah but there are 2 different config files. hard to remember which one has what

Gotcha. Added a comment to make it clearer

turborepo-tests/integration/tests/remote-config.t

…Go code

…nt.Opts. Also refining tests for remote config

turbo-orchestrator bot added the team: turborepo label Apr 10, 2023

NicholasLYang marked this pull request as ready for review April 10, 2023 19:33

NicholasLYang requested a review from a team as a code owner April 10, 2023 19:33

NicholasLYang requested review from mehulkar and chris-olszewski April 10, 2023 19:33

mehulkar approved these changes Apr 10, 2023

View reviewed changes

NicholasLYang force-pushed the nicholasyang/port-config branch from 9eb9e09 to bfe948a Compare April 11, 2023 15:46

chris-olszewski approved these changes Apr 11, 2023

View reviewed changes

NicholasLYang force-pushed the nicholasyang/port-config branch from d131153 to b7472ab Compare April 11, 2023 20:27

NicholasLYang requested a review from a team as a code owner April 21, 2023 21:05

NicholasLYang force-pushed the nicholasyang/port-config branch from f625f4e to 6680c7e Compare April 21, 2023 21:09

turbo-orchestrator bot removed the pkg: turbo-codemod label Apr 21, 2023

mehulkar reviewed Apr 21, 2023

View reviewed changes

cli/internal/cmdutil/cmdutil.go Show resolved Hide resolved

mehulkar reviewed Apr 21, 2023

View reviewed changes

crates/turborepo-lib/src/config/user.rs Show resolved Hide resolved

mehulkar reviewed Apr 21, 2023

View reviewed changes

turborepo-tests/integration/tests/config.t Outdated Show resolved Hide resolved

mehulkar reviewed Apr 21, 2023

View reviewed changes

turborepo-tests/integration/tests/config.t Outdated Show resolved Hide resolved

NicholasLYang requested a review from mehulkar April 24, 2023 20:38

mehulkar approved these changes Apr 24, 2023

View reviewed changes

vercel bot had a problem deploying to Preview – examples-svelte-web April 24, 2023 21:38 Failure

NicholasLYang added 3 commits April 25, 2023 12:28

Added ExecutionState as struct that passes args and remote config to …

1893399

…Go code

Added tests for passing correct config

72bc638

Consoldiating config for remote cache in RemoteConfig instead of clie…

5f97dc0

…nt.Opts. Also refining tests for remote config

NicholasLYang force-pushed the nicholasyang/port-config branch from 578852d to 5f97dc0 Compare April 25, 2023 16:29

NicholasLYang enabled auto-merge (squash) April 25, 2023 16:29

fixed tests and added comment

3271e67

NicholasLYang merged commit 27b654b into vercel:main Apr 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

port(turborepo): Config #4520

port(turborepo): Config #4520

NicholasLYang commented Apr 10, 2023

vercel bot commented Apr 10, 2023 •

edited

Loading

vercel bot commented Apr 10, 2023

mehulkar left a comment

mehulkar Apr 10, 2023

chris-olszewski Apr 11, 2023

NicholasLYang Apr 11, 2023

mehulkar Apr 10, 2023

NicholasLYang Apr 10, 2023

mehulkar Apr 11, 2023

NicholasLYang Apr 11, 2023

chris-olszewski Apr 11, 2023

NicholasLYang Apr 11, 2023

chris-olszewski Apr 11, 2023

chris-olszewski Apr 11, 2023

mehulkar Apr 21, 2023

mehulkar left a comment

mehulkar Apr 24, 2023

NicholasLYang Apr 25, 2023

mehulkar Apr 25, 2023

NicholasLYang Apr 25, 2023

		.add_source(Environment::with_prefix("TURBO").source(environment.clone()))
		.add_source(Environment::with_prefix("VERCEL_ARTIFACTS").source(environment))

port(turborepo): Config #4520

port(turborepo): Config #4520

Conversation

NicholasLYang commented Apr 10, 2023

Description

Testing Instructions

vercel bot commented Apr 10, 2023 • edited Loading

vercel bot commented Apr 10, 2023

mehulkar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mehulkar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Apr 10, 2023 •

edited

Loading