chore(config): fix logic to separate config for WEB_URL and WEB_PORT#147
Closed
dchien234 wants to merge 2 commits into
Closed
chore(config): fix logic to separate config for WEB_URL and WEB_PORT#147dchien234 wants to merge 2 commits into
dchien234 wants to merge 2 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
WEB_URLcurrently does double duty as both the public URL used in links sent to users and the port source used to bind the web server. That works for a local value such ashttp://localhost:8088, but it breaks common reverse-proxy and Kubernetes deployments where the public URL is HTTPS while the application should still bind to an internal port such as8088.Reproduction
Configure Condor behind an ingress or reverse proxy with a public URL and an internal bind port:
Before this change,
utils.configignoredWEB_PORTwheneverWEB_URLwas set and derived the bind port from the URL scheme instead:That means links point to the right public host, but the local web server tries to bind to
443instead of the intended internal port.Fix
Separate the two configuration concerns in
utils/config.py:WEB_URLremains the public URL used in Telegram/web links.WEB_PORTis the local port used to bind the web server.WEB_PORTis set, it always wins for binding.WEB_PORTis not set, the existing fallback behavior remains: use the explicit port fromWEB_URL, infer443for HTTPS, infer80for HTTP, or default tohttp://localhost:8088.Scope
utils/config.py.WEB_URL=http://localhost:8088.WEB_URLor only setWEB_PORT.Validated
Targeted validation passed for the expected config matrix:
WEB_URL=https://condor.example.com,WEB_PORT=8088-> public URL stays HTTPS, bind port is8088.WEB_URL=http://localhost:8088-> bind port remains8088.WEB_URL=https://condor.example.comwithoutWEB_PORT-> bind port falls back to443.WEB_PORT=9090withoutWEB_URL-> public URL becomeshttp://localhost:9090.http://localhost:8088/8088.Also validated:
py_compilepassed forutils/config.py.git diff --checkpassed.Full-suite note:
uv run pytestwas attempted, but collection stops before test execution becausetests/trading_agent/test_risk_drawdown.pyimportscondor.trading_agent, which is not present in this checkout. That failure is outside this one-file config change.