Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

settings: add network.connection_timeout and network.cloud_api.skip_board_detection_calls #2770

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions commands/service_board_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func identifyViaCloudAPI(props *properties.Map, settings *configuration.Settings
}

// identify returns a list of boards checking first the installed platforms or the Cloud API
func identify(pme *packagemanager.Explorer, port *discovery.Port, settings *configuration.Settings) ([]*rpc.BoardListItem, error) {
func identify(pme *packagemanager.Explorer, port *discovery.Port, settings *configuration.Settings, skipCloudAPI bool) ([]*rpc.BoardListItem, error) {
boards := []*rpc.BoardListItem{}
if port.Properties == nil {
return boards, nil
Expand Down Expand Up @@ -170,7 +170,7 @@ func identify(pme *packagemanager.Explorer, port *discovery.Port, settings *conf

// if installed cores didn't recognize the board, try querying
// the builder API if the board is a USB device port
if len(boards) == 0 {
if len(boards) == 0 && !skipCloudAPI && !settings.SkipCloudApiForBoardDetection() {
items, err := identifyViaCloudAPI(port.Properties, settings)
if err != nil {
// this is bad, but keep going
Expand Down Expand Up @@ -225,7 +225,7 @@ func (s *arduinoCoreServerImpl) BoardList(ctx context.Context, req *rpc.BoardLis

ports := []*rpc.DetectedPort{}
for _, port := range dm.List() {
boards, err := identify(pme, port, s.settings)
boards, err := identify(pme, port, s.settings, req.GetSkipCloudApiForBoardDetection())
if err != nil {
warnings = append(warnings, err.Error())
}
Expand Down Expand Up @@ -298,7 +298,7 @@ func (s *arduinoCoreServerImpl) BoardListWatch(req *rpc.BoardListWatchRequest, s

boardsError := ""
if event.Type == "add" {
boards, err := identify(pme, event.Port, s.settings)
boards, err := identify(pme, event.Port, s.settings, req.GetSkipCloudApiForBoardDetection())
if err != nil {
boardsError = err.Error()
}
Expand Down
2 changes: 1 addition & 1 deletion commands/service_board_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func TestBoardIdentifySorting(t *testing.T) {
defer release()

settings := configuration.NewSettings()
res, err := identify(pme, &discovery.Port{Properties: idPrefs}, settings)
res, err := identify(pme, &discovery.Port{Properties: idPrefs}, settings, true)
require.NoError(t, err)
require.NotNil(t, res)
require.Len(t, res, 4)
Expand Down
67 changes: 64 additions & 3 deletions commands/service_settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,12 +224,73 @@ func TestDelete(t *testing.T) {
srv := NewArduinoCoreServer()
loadConfig(t, srv, paths.New("testdata", "arduino-cli.yml"))

_, err := srv.SettingsGetValue(context.Background(), &rpc.SettingsGetValueRequest{Key: "network"})
// Check loaded config
resp, err := srv.ConfigurationSave(context.Background(), &rpc.ConfigurationSaveRequest{
SettingsFormat: "yaml",
})
require.NoError(t, err)
require.YAMLEq(t, `
board_manager:
additional_urls:
- http://foobar.com
- http://example.com

daemon:
port: "50051"

directories:
data: /home/massi/.arduino15
downloads: /home/massi/.arduino15/staging

logging:
file: ""
format: text
level: info

network:
proxy: "123"
`, resp.GetEncodedSettings())

// Check default and setted values
res, err := srv.SettingsGetValue(context.Background(), &rpc.SettingsGetValueRequest{Key: "network"})
require.NoError(t, err)
require.Equal(t, `{"proxy":"123"}`, res.GetEncodedValue())
// Maybe should be like this?
// require.Equal(t, `{"proxy":"123","connection_timeout":"1m0s"}`, res.GetEncodedValue())
res, err = srv.SettingsGetValue(context.Background(), &rpc.SettingsGetValueRequest{Key: "network.connection_timeout"})
require.Equal(t, `"1m0s"`, res.GetEncodedValue()) // default value
require.NoError(t, err)

// Run deletion
_, err = srv.SettingsSetValue(context.Background(), &rpc.SettingsSetValueRequest{Key: "network", EncodedValue: ""})
require.NoError(t, err)
resp, err = srv.ConfigurationSave(context.Background(), &rpc.ConfigurationSaveRequest{
SettingsFormat: "yaml",
})
require.NoError(t, err)
require.YAMLEq(t, `
board_manager:
additional_urls:
- http://foobar.com
- http://example.com

_, err = srv.SettingsGetValue(context.Background(), &rpc.SettingsGetValueRequest{Key: "network"})
require.Error(t, err)
daemon:
port: "50051"

directories:
data: /home/massi/.arduino15
downloads: /home/massi/.arduino15/staging

logging:
file: ""
format: text
level: info
`, resp.GetEncodedSettings())
// Check default and setted values
res, err = srv.SettingsGetValue(context.Background(), &rpc.SettingsGetValueRequest{Key: "network"})
require.NoError(t, err)
require.Equal(t, `{"connection_timeout":"1m0s"}`, res.GetEncodedValue())
res, err = srv.SettingsGetValue(context.Background(), &rpc.SettingsGetValueRequest{Key: "network.connection_timeout"})
require.Equal(t, `"1m0s"`, res.GetEncodedValue()) // default value
require.NoError(t, err)
}
5 changes: 5 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@
[time.ParseDuration()](https://pkg.go.dev/time#ParseDuration), defaults to `720h` (30 days).
- `network` - configuration options related to the network connection.
- `proxy` - URL of the proxy server.
- `connection_timeout` - network connection timeout, the value format must be a valid input for
[time.ParseDuration()](https://pkg.go.dev/time#ParseDuration), defaults to `60s` (60 seconds). `0` means it will
wait indefinitely.
- `cloud_api.skip_board_detection_calls` - if set to `true` it will make the Arduino CLI skip network calls to Arduino
Cloud API to help detection of an unknown board.

### Default directories

Expand Down
41 changes: 31 additions & 10 deletions internal/cli/configuration/configuration.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,8 @@
},
"ttl": {
"description": "cache expiration time of build folders. If the cache is hit by a compilation the corresponding build files lifetime is renewed. The value format must be a valid input for time.ParseDuration(), defaults to `720h` (30 days)",
"oneOf": [
{
"type": "integer",
"minimum": 0
},
{
"type": "string",
"pattern": "^\\+?([0-9]?\\.?[0-9]+(([nuµm]?s)|m|h))+$"
}
]
"type": "string",
"pattern": "^[+-]?(([0-9]+(\\.[0-9]*)?|(\\.[0-9]+))(ns|us|µs|μs|ms|s|m|h))+$"
}
},
"type": "object"
Expand Down Expand Up @@ -146,6 +138,35 @@
},
"type": "object"
},
"network": {
"description": "settings related to network connections.",
"type": "object",
"properties": {
"proxy": {
"description": "proxy settings for network connections.",
"type": "string"
},
"user_agent_ext": {
"description": "extra string to append to the user agent string in HTTP requests.",
"type": "string"
},
"connection_timeout": {
"description": "timeout for network connections, defaults to '30s'",
"type": "string",
"pattern": "^[+-]?(([0-9]+(\\.[0-9]*)?|(\\.[0-9]+))(ns|us|µs|μs|ms|s|m|h))+$"
},
"cloud_api": {
"description": "settings related to the Arduino Cloud API.",
"type": "object",
"properties": {
"skip_board_detection_calls": {
"description": "do not call the Arduino Cloud API to detect an unknown board",
"type": "boolean"
}
}
}
}
},
"output": {
"description": "settings related to text output.",
"properties": {
Expand Down
3 changes: 3 additions & 0 deletions internal/cli/configuration/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ func SetDefaults(settings *Settings) {
// network settings
setKeyTypeSchema("network.proxy", "")
setKeyTypeSchema("network.user_agent_ext", "")
setDefaultValueAndKeyTypeSchema("network.connection_timeout", (time.Second * 60).String())
// network: Arduino Cloud API settings
setKeyTypeSchema("network.cloud_api.skip_board_detection_calls", false)

// locale
setKeyTypeSchema("locale", "")
Expand Down
15 changes: 15 additions & 0 deletions internal/cli/configuration/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"net/url"
"os"
"runtime"
"time"

"github.com/arduino/arduino-cli/commands/cmderrors"
"github.com/arduino/arduino-cli/internal/i18n"
Expand Down Expand Up @@ -58,6 +59,19 @@ func (settings *Settings) ExtraUserAgent() string {
return settings.GetString("network.user_agent_ext")
}

// ConnectionTimeout returns the network connection timeout
func (settings *Settings) ConnectionTimeout() time.Duration {
if timeout, ok, _ := settings.GetDurationOk("network.connection_timeout"); ok {
return timeout
}
return settings.Defaults.GetDuration("network.connection_timeout")
}

// SkipCloudApiForBoardDetection returns whether the cloud API should be ignored for board detection
func (settings *Settings) SkipCloudApiForBoardDetection() bool {
return settings.GetBool("network.cloud_api.skip_board_detection_calls")
}

// NetworkProxy returns the proxy configuration (mainly used by HTTP clients)
func (settings *Settings) NetworkProxy() (*url.URL, error) {
if proxyConfig, ok, _ := settings.GetStringOk("network.proxy"); !ok {
Expand All @@ -82,6 +96,7 @@ func (settings *Settings) NewHttpClient() (*http.Client, error) {
},
userAgent: settings.UserAgent(),
},
Timeout: settings.ConnectionTimeout(),
}, nil
}

Expand Down
39 changes: 39 additions & 0 deletions internal/cli/configuration/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"net/http"
"net/http/httptest"
"testing"
"time"

"github.com/arduino/arduino-cli/internal/cli/configuration"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -68,3 +69,41 @@ func TestProxy(t *testing.T) {
require.NoError(t, err)
require.Equal(t, http.StatusNoContent, response.StatusCode)
}

func TestConnectionTimeout(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
time.Sleep(5 * time.Second)
w.WriteHeader(http.StatusNoContent)
}))
defer ts.Close()

doRequest := func(timeout int) (*http.Response, time.Duration, error) {
settings := configuration.NewSettings()
require.NoError(t, settings.Set("network.proxy", ts.URL))
if timeout != 0 {
require.NoError(t, settings.Set("network.connection_timeout", "2s"))
}
client, err := settings.NewHttpClient()
require.NoError(t, err)

request, err := http.NewRequest("GET", "http://arduino.cc", nil)
require.NoError(t, err)

start := time.Now()
resp, err := client.Do(request)
duration := time.Since(start)

return resp, duration, err
}

// Using default timeout (0), will wait indefinitely (in our case up to 5s)
response, elapsed, err := doRequest(0)
require.NoError(t, err)
require.Equal(t, http.StatusNoContent, response.StatusCode)
require.True(t, elapsed.Seconds() >= 4 && elapsed.Seconds() <= 6) // Adding some tolerance just in case...

// Setting a timeout of 1 seconds, will drop the connection after 1s
_, elapsed, err = doRequest(1)
require.Error(t, err)
require.True(t, elapsed.Seconds() >= 0.5 && elapsed.Seconds() <= 3) // Adding some tolerance just in case...
}
Loading
Loading