Skip to content

Commit

Permalink
Consoldiating config for remote cache in RemoteConfig instead of clie…
Browse files Browse the repository at this point in the history
…nt.Opts. Also refining tests

for remote config
  • Loading branch information
NicholasLYang committed Apr 25, 2023
1 parent 72bc638 commit 5f97dc0
Show file tree
Hide file tree
Showing 13 changed files with 81 additions and 153 deletions.
23 changes: 7 additions & 16 deletions cli/internal/client/client.go
Expand Up @@ -48,34 +48,25 @@ func (c *APIClient) SetToken(token string) {
c.token = token
}

// Opts holds values for configuring the behavior of the API client
type Opts struct {
UsePreflight bool
Timeout uint64
}

// ClientTimeout Exported ClientTimeout used in run.go
const ClientTimeout uint64 = 20

// NewClient creates a new APIClient
func NewClient(remoteConfig turbostate.RemoteConfig, logger hclog.Logger, turboVersion string, opts Opts) *APIClient {
func NewClient(config turbostate.APIClientConfig, logger hclog.Logger, turboVersion string) *APIClient {
client := &APIClient{
baseURL: remoteConfig.APIURL,
baseURL: config.APIURL,
turboVersion: turboVersion,
HTTPClient: &retryablehttp.Client{
HTTPClient: &http.Client{
Timeout: time.Duration(opts.Timeout) * time.Second,
Timeout: time.Duration(config.Timeout) * time.Second,
},
RetryWaitMin: 2 * time.Second,
RetryWaitMax: 10 * time.Second,
RetryMax: 2,
Backoff: retryablehttp.DefaultBackoff,
Logger: logger,
},
token: remoteConfig.Token,
teamID: remoteConfig.TeamID,
teamSlug: remoteConfig.TeamSlug,
usePreflight: opts.UsePreflight,
token: config.Token,
teamID: config.TeamID,
teamSlug: config.TeamSlug,
usePreflight: config.UsePreflight,
}
client.HTTPClient.CheckRetry = client.checkRetry
return client
Expand Down
16 changes: 8 additions & 8 deletions cli/internal/client/client_test.go
Expand Up @@ -31,12 +31,12 @@ func Test_sendToServer(t *testing.T) {
}))
defer ts.Close()

remoteConfig := turbostate.RemoteConfig{
apiClientConfig := turbostate.APIClientConfig{
TeamSlug: "my-team-slug",
APIURL: ts.URL,
Token: "my-token",
}
apiClient := NewClient(remoteConfig, hclog.Default(), "v1", Opts{})
apiClient := NewClient(apiClientConfig, hclog.Default(), "v1")

myUUID, err := uuid.NewUUID()
if err != nil {
Expand Down Expand Up @@ -86,12 +86,12 @@ func Test_PutArtifact(t *testing.T) {
defer ts.Close()

// Set up test expected values
remoteConfig := turbostate.RemoteConfig{
apiClientConfig := turbostate.APIClientConfig{
TeamSlug: "my-team-slug",
APIURL: ts.URL,
Token: "my-token",
}
apiClient := NewClient(remoteConfig, hclog.Default(), "v1", Opts{})
apiClient := NewClient(apiClientConfig, hclog.Default(), "v1")
expectedArtifactBody := []byte("My string artifact")

// Test Put Artifact
Expand All @@ -112,12 +112,12 @@ func Test_PutWhenCachingDisabled(t *testing.T) {
defer ts.Close()

// Set up test expected values
remoteConfig := turbostate.RemoteConfig{
apiClientConfig := turbostate.APIClientConfig{
TeamSlug: "my-team-slug",
APIURL: ts.URL,
Token: "my-token",
}
apiClient := NewClient(remoteConfig, hclog.Default(), "v1", Opts{})
apiClient := NewClient(apiClientConfig, hclog.Default(), "v1")
expectedArtifactBody := []byte("My string artifact")
// Test Put Artifact
err := apiClient.PutArtifact("hash", expectedArtifactBody, 500, "")
Expand All @@ -139,12 +139,12 @@ func Test_FetchWhenCachingDisabled(t *testing.T) {
defer ts.Close()

// Set up test expected values
remoteConfig := turbostate.RemoteConfig{
apiClientConfig := turbostate.APIClientConfig{
TeamSlug: "my-team-slug",
APIURL: ts.URL,
Token: "my-token",
}
apiClient := NewClient(remoteConfig, hclog.Default(), "v1", Opts{})
apiClient := NewClient(apiClientConfig, hclog.Default(), "v1")
// Test Put Artifact
resp, err := apiClient.FetchArtifact("hash")
cd := &util.CacheDisabledError{}
Expand Down
25 changes: 2 additions & 23 deletions cli/internal/cmdutil/cmdutil.go
Expand Up @@ -38,8 +38,6 @@ type Helper struct {

rawRepoRoot string

clientOpts client.Opts

// UserConfigPath is the path to where we expect to find
// a user-specific config file, if one is present. Public
// to allow overrides in tests
Expand Down Expand Up @@ -154,37 +152,19 @@ func (h *Helper) GetCmdBase(executionState *turbostate.ExecutionState) (*CmdBase
if err != nil {
return nil, err
}
remoteConfig := executionState.RemoteConfig
if remoteConfig.Token == "" && ui.IsCI {
vercelArtifactsToken := os.Getenv("VERCEL_ARTIFACTS_TOKEN")
vercelArtifactsOwner := os.Getenv("VERCEL_ARTIFACTS_OWNER")
if vercelArtifactsToken != "" {
remoteConfig.Token = vercelArtifactsToken
}
if vercelArtifactsOwner != "" {
remoteConfig.TeamID = vercelArtifactsOwner
}
}

// Primacy: Arg > Env
timeout, err := executionState.CLIArgs.GetRemoteCacheTimeout()
if err == nil {
h.clientOpts.Timeout = timeout
}
apiClientConfig := executionState.APIClientConfig

apiClient := client.NewClient(
remoteConfig,
apiClientConfig,
logger,
h.TurboVersion,
h.clientOpts,
)

return &CmdBase{
UI: terminal,
Logger: logger,
RepoRoot: repoRoot,
APIClient: apiClient,
RemoteConfig: remoteConfig,
TurboVersion: h.TurboVersion,
}, nil
}
Expand All @@ -195,7 +175,6 @@ type CmdBase struct {
Logger hclog.Logger
RepoRoot turbopath.AbsoluteSystemPath
APIClient *client.APIClient
RemoteConfig turbostate.RemoteConfig
TurboVersion string
}

Expand Down
14 changes: 9 additions & 5 deletions cli/internal/cmdutil/cmdutil_test.go
Expand Up @@ -5,17 +5,19 @@ import (
"testing"
"time"

"github.com/vercel/turbo/cli/internal/fs"
"github.com/vercel/turbo/cli/internal/turbostate"
"gotest.tools/v3/assert"
)

func TestRemoteCacheTimeoutFlag(t *testing.T) {
args := turbostate.ParsedArgsFromRust{
CWD: "",
RemoteCacheTimeout: 599,
CWD: "",
}

executionState := turbostate.ExecutionState{
APIClientConfig: turbostate.APIClientConfig{
Timeout: 599,
},
CLIArgs: args,
}

Expand All @@ -38,10 +40,12 @@ func TestRemoteCacheTimeoutPrimacy(t *testing.T) {
_ = os.Unsetenv(key)
})
args := turbostate.ParsedArgsFromRust{
CWD: "",
RemoteCacheTimeout: 1,
CWD: "",
}
executionState := turbostate.ExecutionState{
APIClientConfig: turbostate.APIClientConfig{
Timeout: 1,
},
CLIArgs: args,
}
h := NewHelper("test-version", &args)
Expand Down
2 changes: 0 additions & 2 deletions cli/internal/run/run.go
Expand Up @@ -66,8 +66,6 @@ func optsFromArgs(args *turbostate.ParsedArgsFromRust) (*Opts, error) {
return nil, err
}

// Cache flags
opts.clientOpts.Timeout = args.RemoteCacheTimeout
opts.cacheOpts.SkipFilesystem = runPayload.RemoteOnly
opts.cacheOpts.OverrideDir = runPayload.CacheDir
opts.cacheOpts.Workers = runPayload.CacheWorkers
Expand Down
5 changes: 0 additions & 5 deletions cli/internal/run/run_spec.go
Expand Up @@ -6,7 +6,6 @@ import (
"strings"

"github.com/vercel/turbo/cli/internal/cache"
"github.com/vercel/turbo/cli/internal/client"
"github.com/vercel/turbo/cli/internal/runcache"
"github.com/vercel/turbo/cli/internal/scope"
"github.com/vercel/turbo/cli/internal/util"
Expand Down Expand Up @@ -42,7 +41,6 @@ func (rs *runSpec) ArgsForTask(task string) []string {
type Opts struct {
runOpts util.RunOpts
cacheOpts cache.Opts
clientOpts client.Opts
runcacheOpts runcache.Opts
scopeOpts scope.Opts
}
Expand Down Expand Up @@ -83,8 +81,5 @@ func getDefaultOptions() *Opts {
runOpts: util.RunOpts{
Concurrency: 10,
},
clientOpts: client.Opts{
Timeout: client.ClientTimeout,
},
}
}
35 changes: 10 additions & 25 deletions cli/internal/turbostate/turbostate.go
Expand Up @@ -4,18 +4,9 @@
package turbostate

import (
"fmt"

"github.com/vercel/turbo/cli/internal/util"
)

// RepoState is the state for repository. Consists of the root for the repo
// along with the mode (single package or multi package)
type RepoState struct {
Root string `json:"root"`
Mode string `json:"mode"`
}

// DaemonPayload is the extra flags and command that are
// passed for the `daemon` subcommand
type DaemonPayload struct {
Expand Down Expand Up @@ -98,22 +89,16 @@ type ParsedArgsFromRust struct {

// ExecutionState is the entire state of a turbo execution that is passed from the Rust shim.
type ExecutionState struct {
RemoteConfig RemoteConfig `json:"remote_config"`
CLIArgs ParsedArgsFromRust `json:"cli_args"`
}

// RemoteConfig holds the authentication and endpoint details for the API client
type RemoteConfig struct {
Token string `json:"token"`
TeamID string `json:"team_id"`
TeamSlug string `json:"team_slug"`
APIURL string `json:"api_url"`
APIClientConfig APIClientConfig `json:"remote_config"`
CLIArgs ParsedArgsFromRust `json:"cli_args"`
}

// GetRemoteCacheTimeout returns the value of the `remote-cache-timeout` flag.
func (a ParsedArgsFromRust) GetRemoteCacheTimeout() (uint64, error) {
if a.RemoteCacheTimeout != 0 {
return a.RemoteCacheTimeout, nil
}
return 0, fmt.Errorf("no remote cache timeout provided")
// APIClientConfig holds the authentication and endpoint details for the API client
type APIClientConfig struct {
Token string `json:"token"`
TeamID string `json:"team_id"`
TeamSlug string `json:"team_slug"`
APIURL string `json:"api_url"`
UsePreflight bool `json:"use_preflight"`
Timeout uint64 `json:"timeout"`
}
15 changes: 6 additions & 9 deletions crates/turborepo-api-client/src/lib.rs
Expand Up @@ -308,16 +308,13 @@ impl APIClient {
false
}

pub fn new(
base_url: impl AsRef<str>,
timeout: Option<u64>,
version: &'static str,
) -> Result<Self> {
let client = match timeout {
Some(timeout) => reqwest::Client::builder()
pub fn new(base_url: impl AsRef<str>, timeout: u64, version: &'static str) -> Result<Self> {
let client = if timeout != 0 {
reqwest::Client::builder()
.timeout(std::time::Duration::from_secs(timeout))
.build()?,
None => reqwest::Client::builder().build()?,
.build()?
} else {
reqwest::Client::builder().build()?
};

let user_agent = format!(
Expand Down
8 changes: 1 addition & 7 deletions crates/turborepo-lib/src/cli.rs
Expand Up @@ -107,13 +107,7 @@ pub struct Args {
#[clap(long, global = true)]
pub preflight: bool,
/// Set a timeout for all HTTP requests.
#[clap(
long,
env = "TURBO_REMOTE_CACHE_TIMEOUT",
value_name = "TIMEOUT",
global = true,
value_parser
)]
#[clap(long, value_name = "TIMEOUT", global = true, value_parser)]
pub remote_cache_timeout: Option<u64>,
/// Set the team slug for API calls
#[clap(long, global = true, value_parser)]
Expand Down
21 changes: 4 additions & 17 deletions crates/turborepo-lib/src/config/client.rs
Expand Up @@ -13,7 +13,7 @@ pub struct ClientConfig {

#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)]
struct ClientConfigValue {
remote_cache_timeout: Option<u64>,
remote_cache_timeout: u64,
}

#[derive(Debug, Clone)]
Expand All @@ -23,24 +23,13 @@ pub struct ClientConfigLoader {
}

impl ClientConfig {
#[allow(dead_code)]
pub fn remote_cache_timeout(&self) -> Option<u64> {
match self.config.remote_cache_timeout {
// Pass 0 to get no timeout.
Some(0) => None,

// Pass any non-zero uint64 to get a timeout of that duration measured in seconds.
Some(other) => Some(other),

// If the _config_ doesn't have a remote_cache_timeout, give them the default.
None => Some(DEFAULT_TIMEOUT),
}
pub fn remote_cache_timeout(&self) -> u64 {
self.config.remote_cache_timeout
}
}

impl ClientConfigLoader {
/// Creates a loader that will load the client config
#[allow(dead_code)]
pub fn new() -> Self {
Self {
remote_cache_timeout: None,
Expand All @@ -49,7 +38,6 @@ impl ClientConfigLoader {
}

/// Set an override for token that the user provided via the command line
#[allow(dead_code)]
pub fn with_remote_cache_timeout(mut self, remote_cache_timeout: Option<u64>) -> Self {
self.remote_cache_timeout = remote_cache_timeout;
self
Expand All @@ -61,7 +49,6 @@ impl ClientConfigLoader {
self
}

#[allow(dead_code)]
pub fn load(self) -> Result<ClientConfig> {
let Self {
remote_cache_timeout,
Expand All @@ -79,7 +66,7 @@ impl ClientConfigLoader {
match config_attempt {
Err(_) => Ok(ClientConfig {
config: ClientConfigValue {
remote_cache_timeout: None,
remote_cache_timeout: 0,
},
}),
Ok(config) => Ok(ClientConfig { config }),
Expand Down

0 comments on commit 5f97dc0

Please sign in to comment.