Skip to content

Add --publish flag for Render publish-only deployment#52

Open
IamJasonBian wants to merge 3 commits intomainfrom
IamJasonBian/render-prod-dry-run-v1
Open

Add --publish flag for Render publish-only deployment#52
IamJasonBian wants to merge 3 commits intomainfrom
IamJasonBian/render-prod-dry-run-v1

Conversation

@IamJasonBian
Copy link
Copy Markdown
Owner

Summary

  • Add --publish flag to decouple blob uploads from live trading, enabling Render deployments that update the site without placing trades
  • Skip broker initialization in publish-only mode to allow headless operation
  • Add render.yaml configuration for allocation-engine-2.0 Background Worker

Changes

  • trading_system/main.py: Add --publish parameter, skip broker init when publish-only, decouple blob upload from --live flag
  • render.yaml (new): Worker configuration with start command --publish --dashboard --continuous
  • public/dashboard/ (new): Dashboard HTML and GIF assets

Blob uploads now gated by live=not self.dry_run or self.publish, allowing site updates without live trading.

Allows running the engine in dry-run mode (no Robinhood orders) while
still uploading state to Netlify Blobs, enabling the dashboard at
5thstreetcapital.org/trade. Includes render.yaml for Render deployment.
When --publish is used without --live, the engine only needs market data
and blob uploads — no Robinhood credentials required. Guards all
trading_bot calls in run_once() so the service can run without
RH_AUTOMATED_ACCOUNT_NUMBER.
Copy link
Copy Markdown
Owner Author

Code Review — PR #52

Overall: Clean implementation of publish-only mode. The broker skip logic and null guards are well-placed.

Issues

  1. render.yaml exposes ticker strategy in start command — The startCommand hard-codes --ticker BTC SPY QQQ AMZN. Consider using an env var (TICKER_LIST) to avoid redeploying just to change symbols.

  2. Publish + live interaction is unclear — The condition not (self.publish and self.dry_run) means --publish --live still initializes the broker. The --publish flag description says "Upload blobs in dry-run mode" but nothing enforces mutual exclusivity with --live. Consider adding a guard:

    if args.publish and args.live:
        parser.error("--publish and --live are mutually exclusive")
  3. portfolio_data can be None downstream — At line ~699, portfolio_data is set to None when self.trading_bot is None, but log_state_to_blob and _send_oncall_summary receive it. Verify those functions handle portfolio=None gracefully.

Nits

  • The Slack deploy message correctly shows "PUBLISH" mode — nice touch.
  • render.yaml env vars all have sync: false — confirm this is intentional (secrets won't auto-sync across Render services).

Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Follow-up Review — PR #52

Additional observations not covered in the initial review:

  1. Broker-skip condition + render.yaml = latent bug — The condition not (self.publish and self.dry_run) requires both flags to skip broker init. But render.yaml's start command uses --publish without --dry-run, meaning the Render worker will still attempt to initialize the broker. Either the condition should be not self.publish (if publish is always headless), or render.yaml needs --dry-run added.

  2. render.yaml declares unused RH credentialsRH_USERNAME, RH_PASSWORD, RH_TOTP_CODE are declared as env vars for a publish-only worker that shouldn't need broker access. This creates confusion about what secrets are actually required for this service.

  3. Merge conflicts exist — The PR's mergeable state is dirty. Needs a rebase onto current main before it can be merged.


Generated by Claude Code

@IamJasonBian IamJasonBian added the stale label Apr 11, 2026 — with Claude
Copy link
Copy Markdown
Owner Author

Stale PR notice — This PR has been open for 18 days with no new commits since Mar 24. Two code reviews flagged a latent bug (broker init not skipped without --dry-run), unused RH credentials in render.yaml, and merge conflicts with main. Please rebase and address review feedback or close if no longer needed.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Stale — 4 weeks, no activity since Mar 24. Two prior reviews flagged a latent bug (broker init not actually skipped without --dry-run), unused RH credentials in render.yaml, and merge conflicts with main. The mergeable state was already dirty as of Apr 4. Please rebase and fix the broker-skip condition, or close if the Render deployment approach has changed.


Generated by Claude Code

In publish-only/headless Render deploys, trading_bot is None when RH
credentials are absent. _send_oncall_summary called .get_pdt_status()
unconditionally, raising AttributeError after every successful blob
upload. The existing 'PDT: unavailable' branch already handles None,
so the fix is a single ternary guard.

Co-authored-by: jasonzb <jason.bian75@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant