Make pact-proxy more configurable using ENV variables #27
Conversation
|
|
||
| func ConfigureProxy(config ProxyConfig) error { | ||
| targetURL, err := url.Parse(config.Target) | ||
| var c pactproxy.Config |
internal/app/pactproxy/proxy.go
Outdated
|
|
||
| func (c *Config) SetDefaults() { | ||
| defaultConfig := Config{} | ||
| // Can be changed (on next release) to: envconfig.ExtractDefaults(context.Background(), &defaultConfig) |
There was a problem hiding this comment.
should we carry this information in a issue ticket instead of a comment?
There was a problem hiding this comment.
Yes, but I've decided to remove SetDefaults in favor of a simpler approach
internal/app/pactproxy/proxy.go
Outdated
| retryFor(func(timeLeft time.Duration) bool { | ||
| log.Infof("retry: %s", timeLeft) |
internal/app/pactproxy/proxy.go
Outdated
| @@ -179,7 +200,9 @@ func (a *api) interactionsWaitHandler(res http.ResponseWriter, req *http.Request | |||
| } | |||
|
|
|||
| log.Infof("waiting for %s", waitFor) | |||
internal/app/configuration/api.go
Outdated
| err = json.Unmarshal(configBytes, &proxyConfig) | ||
| if err != nil { | ||
| httpresponse.Errorf(w, http.StatusBadRequest, "unable to parse interactionConstraint from data. %s", err.Error()) | ||
| return | ||
| } | ||
| proxyConfig.SetDefaults() |
There was a problem hiding this comment.
Little bit confused about this. We first unmarshal proxyConfig from the payload, we then invoke SetDefaults which creates a new Struct, and runs envconfig.ProcessWith(context.Background(), &defaultConfig, envconfig.MapLookuper(nil)) . does this mean we ignore the unmarshalled result?
There was a problem hiding this comment.
This is a bit horrible. I think this admin handler is only used by the tests. Because the config is posted as json, the env loading can't be used. Any config values not passed will be set to zero values.
proxyConfig.SetDefaults() is updating the struct and overwriting zero values with the default values, for fields where this makes sense (AdminPort, WaitDelay, WaitDuration). Tests fail if WaitDelay is zero.
There was a problem hiding this comment.
I have removed the SetDefaults as it is indeed confusing. Moved back to a simpler handling of defaults in the proxy: define constants and a simple if to set them if passed values are zero.
| serverAddrURL, err := url.Parse(serverAddr) | ||
| if err != nil { | ||
| return err | ||
| log.Fatal(err.Error()) |
There was a problem hiding this comment.
I would prefer if the function returned pactproxy.Config, error and we did the fatal in main.go.
Work as part of investment time: https://github.com/form3tech/investment-time-project/issues/62
Collect configuration into one struct.
Add configuration for the WaitDelay and WaitDuration
Add envconfig library to tidy up parsing of env vars