Add background portfolio GUI service#30
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Adds a background-management path for the local portfolio GUI so users can start, stop, and query a localhost service instead of only running the dashboard in the foreground.
Changes:
- Adds a new
trading_lab.portfolio.servicemodule withstart,stop,status, andservecommands for the GUI service. - Wires top-level CLI shortcuts (
tl start,tl stop,tl service ...) into the maintlentrypoint. - Updates the GUI launcher defaults to use port
811and support suppressing automatic browser open for the background worker.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
src/trading_lab/portfolio/service.py |
New background service manager for the portfolio GUI, including PID/log handling and CLI entrypoints. |
src/trading_lab/portfolio/gui.py |
Updates GUI defaults for the new service flow and adds optional browser suppression. |
src/trading_lab/cli/main.py |
Adds top-level command dispatch for the new service shortcuts. |
tests/test_portfolio_service.py |
Adds initial tests for service defaults, missing PID handling, stop cleanup, and shortcut presence. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+210
to
+213
| for name in ("start", "stop", "status", "serve"): | ||
| p = sub.add_parser(name) | ||
| p.add_argument("--host", default=DEFAULT_HOST) | ||
| p.add_argument("--port", type=int, default=DEFAULT_PORT) |
Comment on lines
+162
to
+173
| pid = _read_pid() | ||
| if pid and _pid_is_running(pid): | ||
| try: | ||
| if platform.system().lower() == "windows": | ||
| subprocess.run( | ||
| ["taskkill", "/PID", str(pid), "/T", "/F"], | ||
| stdout=subprocess.DEVNULL, | ||
| stderr=subprocess.DEVNULL, | ||
| check=False, | ||
| ) | ||
| else: | ||
| os.kill(pid, signal.SIGTERM) |
|
|
||
| def status(host: str = DEFAULT_HOST, port: int = DEFAULT_PORT) -> ServiceStatus: | ||
| pid = _read_pid() | ||
| running = bool(pid and _pid_is_running(pid)) or _server_responds(host, port) |
Comment on lines
+70
to
+74
| def _server_responds(host: str = DEFAULT_HOST, port: int = DEFAULT_PORT) -> bool: | ||
| try: | ||
| with urlopen(service_url(host, port), timeout=1.5) as response: | ||
| return 200 <= int(response.status) < 500 | ||
| except (OSError, URLError, TimeoutError): |
Comment on lines
+129
to
+130
| if current.pid and force: | ||
| stop(host, port) |
Comment on lines
+221
to
+225
| print_status(start(args.host, args.port, open_browser=not args.no_open, force=args.force)) | ||
| return 0 | ||
| if args.cmd == "stop": | ||
| print_status(stop(args.host, args.port)) | ||
| return 0 |
Comment on lines
+155
to
+157
| final = status(host, port) | ||
| if open_browser: | ||
| webbrowser.open(final.url) |
Comment on lines
+14
to
+18
| if argv_list and argv_list[0] in {"start", "stop", "service"}: | ||
| from trading_lab.portfolio import service as _tl_service | ||
|
|
||
| if argv_list[0] == "service": | ||
| raise SystemExit(_tl_service.main(argv_list[1:] or ["status"])) |
Comment on lines
+43
to
+47
| def test_cli_has_top_level_service_shortcuts(): | ||
| import trading_lab.cli.main as cli_main | ||
|
|
||
| text = Path(cli_main.__file__).read_text(encoding="utf-8") | ||
| assert '"start", "stop", "service"' in text |
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.
Adds local-only tl start, tl stop, and tl service status commands for running the portfolio GUI in the background on 127.0.0.1:811 across supported operating systems.