Port can be defined by env var #244
Conversation
Codecov Report
@@ Coverage Diff @@
## main #244 +/- ##
==========================================
+ Coverage 54.79% 54.80% +0.01%
==========================================
Files 86 86
Lines 3581 3586 +5
==========================================
+ Hits 1962 1965 +3
- Misses 1415 1416 +1
- Partials 204 205 +1
Continue to review full report at Codecov.
|
pkg/cli/cli.go
Outdated
| port := os.Getenv("PORT") | ||
| if port != "" { | ||
| cfg.Server.APIBindAddr = fmt.Sprintf(":%s", port) | ||
| cfg.Agent.ServerAddress = fmt.Sprintf("http://localhost:%s", port) | ||
| cfg.Exec.ServerAddress = fmt.Sprintf("http://localhost:%s", port) | ||
| } | ||
|
|
There was a problem hiding this comment.
Some points for your consideration:
ffcliis used to bind environment variables to configuration (and we have some plans to migrate to cobra - see Pyroscope Process Lifecycle Refactoring #240).- This affects how CLI usage message (and documentation) is rendered: Improves generate-sample-config script #236.
- To keep things in order, it's better to preserve 1:1 mapping between CLI args, env vars, and configuration options (wherever it is possible).
To be very honest, I'm not 100% sure this option is required indeed.
There was a problem hiding this comment.
Thanks for the useful remarks. I wasn't aware of ffcli. I guess we need a decision if this is needed at all here. There might be cases when we need to have port configurable.
|
Let's not merge this, you can use |
No description provided.