Skip to content

Commit

Permalink
refactor: Make base URL a required parameter in client constructor
Browse files Browse the repository at this point in the history
- Enforce base URL as mandatory in constructor; removes the illusion
of it being an 'optional' setting when it's fundamentally required for operation
  • Loading branch information
frrist committed Jan 26, 2024
1 parent 2954543 commit e59b278
Show file tree
Hide file tree
Showing 10 changed files with 20 additions and 37 deletions.
4 changes: 1 addition & 3 deletions cmd/cli/serve/serve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,7 @@ func (s *ServeSuite) TestCanSubmitJob() {
docker.MustHaveDocker(s.T())
port, _ := s.serve("--node-type", "requester", "--node-type", "compute")
client := client.NewAPIClient(client.NoTLS, "localhost", port)
clientV2 := clientv2.New(clientv2.Config{
Address: fmt.Sprintf("http://127.0.0.1:%d", port),
})
clientV2 := clientv2.New(fmt.Sprintf("http://127.0.0.1:%d", port))
s.Require().NoError(apitest.WaitForAlive(s.ctx, clientV2))

job, err := model.NewJobWithSaneProductionDefaults()
Expand Down
4 changes: 1 addition & 3 deletions cmd/cli/serve/timeout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,7 @@ func (s *ServeSuite) TestNoTimeoutSetOrApplied() {
s.Require().NoError(err)

client := client.NewAPIClient(client.NoTLS, "localhost", port)
clientV2 := clientv2.New(clientv2.Config{
Address: fmt.Sprintf("http://127.0.0.1:%d", port),
})
clientV2 := clientv2.New(fmt.Sprintf("http://127.0.0.1:%d", port))
s.Require().NoError(apitest.WaitForAlive(s.ctx, clientV2))

testJob := model.NewJob()
Expand Down
4 changes: 1 addition & 3 deletions cmd/testing/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@ func (s *BaseSuite) SetupTest() {
s.Host = s.Node.APIServer.Address
s.Port = s.Node.APIServer.Port
s.Client = client.NewAPIClient(client.NoTLS, s.Host, s.Port)
s.ClientV2 = clientv2.New(clientv2.Config{
Address: fmt.Sprintf("http://%s:%d", s.Host, s.Port),
})
s.ClientV2 = clientv2.New(fmt.Sprintf("http://%s:%d", s.Host, s.Port))
}

// After each test
Expand Down
2 changes: 1 addition & 1 deletion cmd/util/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,5 @@ func GetAPIClientV2() *clientv2.Client {
}))
}

return clientv2.New(clientv2.Config{Address: base}, opts...)
return clientv2.New(base, opts...)
}
4 changes: 1 addition & 3 deletions ops/aws/canary/lambda/pkg/scenarios/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,7 @@ func getClient() *client.APIClient {

func getClientV2() *clientv2.Client {
host, port := getClientHostAndPort()
return clientv2.New(clientv2.Config{
Address: fmt.Sprintf("http://%s:%d", host, port),
})
return clientv2.New(fmt.Sprintf("http://%s:%d", host, port))
}

func getNodeSelectors() ([]model.LabelSelectorRequirement, error) {
Expand Down
13 changes: 11 additions & 2 deletions pkg/publicapi/client/v2/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,24 @@ import (
)

type Client struct {
address string

httpClient *http.Client
config Config
}

// New creates a new client.
func New(cfg Config, optFns ...OptionFn) *Client {
func New(address string, optFns ...OptionFn) *Client {
// define default filed on the config by setting them here, then
// modify with options to override.
var cfg Config
for _, optFn := range optFns {
optFn(&cfg)
}

resolveHTTPClient(&cfg)
return &Client{
address: address,
httpClient: cfg.HTTPClient,
config: cfg,
}
Expand Down Expand Up @@ -178,7 +184,10 @@ func (c *Client) toHTTP(ctx context.Context, method, endpoint string, r *apimode

// generate URL for a given endpoint
func (c *Client) url(endpoint string) (*url.URL, error) {
base, _ := url.Parse(c.config.Address)
base, err := url.Parse(c.address)
if err != nil {
return nil, err
}
u, err := url.Parse(endpoint)
if err != nil {
return nil, err
Expand Down
10 changes: 0 additions & 10 deletions pkg/publicapi/client/v2/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ import (
)

type Config struct {
// Address is the address of the node's public REST API.
Address string

// Namespace is the default namespace to use for all requests.
Namespace string

Expand Down Expand Up @@ -80,13 +77,6 @@ func WithInsecureTLS(insecure bool) OptionFn {
}
}

// WithAddress sets the address of the node's public REST API.
func WithAddress(address string) OptionFn {
return func(o *Config) {
o.Address = address
}
}

// WithNamespace sets the default namespace to use for all requests.
func WithNamespace(namespace string) OptionFn {
return func(o *Config) {
Expand Down
8 changes: 2 additions & 6 deletions pkg/publicapi/test/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,10 @@ func (s *ServerSuite) SetupSuite() {
)

s.requesterNode = stack.Nodes[0]
s.client = client.New(client.Config{
Address: s.requesterNode.APIServer.GetURI().String(),
})
s.client = client.New(s.requesterNode.APIServer.GetURI().String())

s.computeNode = stack.Nodes[1]
s.computeClient = client.New(client.Config{
Address: s.computeNode.APIServer.GetURI().String(),
})
s.computeClient = client.New(s.computeNode.APIServer.GetURI().String())

s.Require().NoError(WaitForAlive(ctx, s.client))
s.Require().NoError(WaitForAlive(ctx, s.computeClient))
Expand Down
4 changes: 1 addition & 3 deletions pkg/publicapi/test/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,7 @@ func setupNodeForTestWithConfig(t *testing.T, apiCfg publicapi.Config) (*node.No
err = n.Start(ctx)
require.NoError(t, err)

apiClient := client.New(client.Config{
Address: n.APIServer.GetURI().String(),
})
apiClient := client.New(n.APIServer.GetURI().String())
require.NoError(t, WaitForNodes(ctx, apiClient))
return n, apiClient
}
4 changes: 1 addition & 3 deletions pkg/test/scenario/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,7 @@ func (s *ScenarioRunner) RunScenario(scenario Scenario) string {

apiServer := stack.Nodes[0].APIServer
apiClient := client.NewAPIClient(client.NoTLS, apiServer.Address, apiServer.Port)
apiClientV2 := clientv2.New(clientv2.Config{
Address: fmt.Sprintf("http://%s:%d", apiServer.Address, apiServer.Port),
})
apiClientV2 := clientv2.New(fmt.Sprintf("http://%s:%d", apiServer.Address, apiServer.Port))

submittedJob, submitError := apiClient.Submit(s.Ctx, j)
if scenario.SubmitChecker == nil {
Expand Down

0 comments on commit e59b278

Please sign in to comment.