RSDK-10704, RSDK-10292: Config: new setting viam_server_env to pass environment variables to viam-server #116
Conversation
aldenh-viam
changed the title
cheukt
requested review from
benjirewis and
jmatth
and removed request for
benjirewis
cheukt
left a comment
cheukt
left a comment
There was a problem hiding this comment.
does the viam-server restart if you update env vars on app.viam.com?
I don't believe it does (for any config change). It gets downloaded, but only restarts if there's a new EDIT: Discussed offline. Added in 0359a8a . It will only restart if it detects a change in the new "viam_server_env" section, but not if other options have changed. |
|
Added auto restart: #116 (comment) |
jmatth
left a comment
jmatth
left a comment
There was a problem hiding this comment.
Just minor comments from me, optimistically LGTM.
| DisableNetworkConfiguration: Tribool(0), | ||
| DisableSystemConfiguration: Tribool(0), | ||
| ViamServerStartTimeoutMinutes: Timeout(time.Minute * 10), | ||
| ViamServerExtraEnvVars: nil, |
There was a problem hiding this comment.
nit: nil is the empty value for map in go, you don't need this line here.
There was a problem hiding this comment.
You don't NEED it, but you don't need a lot of the other zero-values in this block either. They're here to make code more understandable, and to guard against any accidental change of type(s) in the future. E.g. it's easier to look at a "complete" structure here than see only the non-zero fields.
tl;dr I'd prefer it stay.
| defer s.mu.Unlock() | ||
| s.startTimeout = time.Duration(cfg.AdvancedSettings.ViamServerStartTimeoutMinutes) | ||
|
|
||
| if !reflect.DeepEqual(cfg.AdvancedSettings.ViamServerExtraEnvVars, s.extraEnvVars) { |
There was a problem hiding this comment.
Prefer the new maps.Equal over reflect. It should accomplish the same thing with better performance in situations like this where we know all the types.
🔗 Link your GitHub account to AtlassianTo enable Code Reviewer, please link your GitHub account to your Atlassian account. Click here to connect your accounts This is a one-time setup that takes less than a minute. |
4 participants
The
advanced_settingssection of the config now supportsviam_server_envto pass user-specified environment variables toviam-serverand its children.Example:
Keys and values must both be strings. To unset, remove from the mapping.
Note all of the existing environment variables from Agent's current environment currently also gets passed to
viam-server, e.g. HOME, PWD, TERM, PATH, etc.