Skip to content
New issue

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

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

Already on GitHub? # to your account

Config file options not used #42

Closed
devusb opened this issue Jan 7, 2024 · 3 comments · Fixed by #43
Closed

Config file options not used #42

devusb opened this issue Jan 7, 2024 · 3 comments · Fixed by #43

Comments

@devusb
Copy link

devusb commented Jan 7, 2024

When running with a config file as

$ go-simple-upload-server -config config.json 

where config.json contains

{
  "addr": "127.0.0.1:8081"
}

I will get the following error (because 8080 is in use for something else on my system)

2024/01/07 12:20:07 Start listening on 127.0.0.1:8080
2024/01/07 12:20:07 server stopped: unable to listen on 127.0.0.1:8080: listen tcp 127.0.0.1:8080: bind: address already in use

But, if I intentionally set addr to an empty string (so bypass triggering of the default flag value), it works:

$ go-simple-upload-server -addr "" -config config.json
2024/01/07 12:21:32 Start listening on 127.0.0.1:8081
2024/01/07 12:21:32 Start serving on 127.0.0.1:8081

Thinking this is related to the merging strategy used to combine flags and the config file, because if I remove the WithOverride strategy the config file works:

diff --git a/app.go b/app.go
index afaf1e9..7b4b279 100644
--- a/app.go
+++ b/app.go
@@ -50,7 +50,7 @@ func main() {
 		if err := json.NewDecoder(f).Decode(&fileConfig); err != nil {
 			log.Fatalf("failed to load config: %v", err)
 		}
-		if err := mergo.Merge(&fileConfig, config, mergo.WithOverride); err != nil {
+		if err := mergo.Merge(&fileConfig, config); err != nil {
 			log.Fatalf("failed to merge config: %v", err)
 		}
 		config = fileConfig

with the consequence that config file options will always take precedence over command line options, which is not entirely intuitive.

@mayth
Copy link
Owner

mayth commented Feb 14, 2024

thank you for the report. I'm working for this.

@mayth mayth closed this as completed in #43 Mar 24, 2024
mayth added a commit that referenced this issue Mar 24, 2024
@mayth
Copy link
Owner

mayth commented Mar 24, 2024

@devusb
This might be fixed in #43.

$ cat config.json
{
    "addr": ":8088"
}

$ ./go-simple-upload-server -config config.json
...
2024/03/24 20:23:34 Start listening on :8088

@devusb
Copy link
Author

devusb commented Mar 29, 2024

Thanks for the fix, and also the project!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants