Skip to content

feat(server): Add skip-standby-site-creation flag#6678

Open
regdocs wants to merge 1 commit into
developfrom
feat/server-skip-standby-site-creation
Open

feat(server): Add skip-standby-site-creation flag#6678
regdocs wants to merge 1 commit into
developfrom
feat/server-skip-standby-site-creation

Conversation

@regdocs

@regdocs regdocs commented Jun 11, 2026

Copy link
Copy Markdown
Member

Adds a Skip Standby Site Creation Check field to the Server doctype. When enabled, the scheduler will not pick that server when replenishing standby site pools.

The flag is respected in all three pool systems:

  • product_trial.get_server_from_cluster (product trial pools)
  • saas_site.get_saas_bench (legacy SaaS app pools)
  • erpnext_site.get_erpnext_bench (ERPNext site pool)

Adds a `Skip Standby Site Creation` Check field to the Server doctype.
When enabled, the scheduler will not pick that server when replenishing
standby site pools.

The flag is respected in all three pool systems:
- `product_trial.get_server_from_cluster` (product trial pools)
- `saas_site.get_saas_bench` (legacy SaaS app pools)
- `erpnext_site.get_erpnext_bench` (ERPNext site pool)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 4/5

Safe to merge for the two ORM/raw-SQL paths; saas_site.get_saas_bench can crash if an operator marks every eligible server as skipped.

The flag is applied consistently and the schema change is correct. The only gap is in saas_site.py where an empty result from the new filter reaches an unguarded index access, turning a theoretical edge case into something an operator can accidentally trigger.

press/press/doctype/site/saas_site.py — the get_saas_bench function needs a guard before accessing signup_servers[0].

Important Files Changed

Filename Overview
press/press/doctype/server/server.json Adds skip_standby_site_creation Check field (default 0) — schema change is correct and straightforward.
press/press/doctype/server/server.py Adds typed annotation skip_standby_site_creation: DF.Check — matches the JSON schema addition, no issues.
press/press/doctype/site/erpnext_site.py Appends AND server.skip_standby_site_creation = 0 to existing parameterized raw SQL — literal value, no injection risk.
press/press/doctype/site/saas_site.py Adds filter to raw SQL, but empty result set (all servers flagged) causes IndexError at line 117 — pre-existing flaw now operator-triggerable.
press/saas/doctype/product_trial/product_trial.py Uses query builder Server.skip_standby_site_creation == 0 correctly; also reformats a get_all call with no logic change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Standby Site Pool Replenishment] --> B{Which pool system?}
    B --> C[product_trial.get_server_from_cluster]
    B --> D[saas_site.get_saas_bench]
    B --> E[erpnext_site.get_erpnext_bench]

    C --> F[frappe.qb WHERE Server.skip_standby_site_creation == 0]
    D --> G[SQL AND server.skip_standby_site_creation = 0]
    E --> H[SQL AND server.skip_standby_site_creation = 0]

    F --> I[Eligible servers list]
    G --> J{bench_servers empty?}
    H --> K[Return bench name]

    J -->|No| L[Build signup_server_sub_str → select lowest CPU server]
    J -->|Yes| M[⚠️ IndexError: signup_servers index 0 out of range]
    I --> N[Count standby sites per server → pick min]
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
press/press/doctype/site/saas_site.py:117
**IndexError when all servers are skipped**

If `skip_standby_site_creation` is enabled on every server that would otherwise match the query, `bench_servers` will be empty, making `signup_servers` an empty tuple. The `else` branch on this line then accesses `signup_servers[0]`, raising an `IndexError`. Before this PR the same crash was theoretically possible, but the new flag makes it a realistic operator-triggered failure path (e.g., a DBA flags all servers in a cluster during maintenance).

Reviews (1): Last reviewed commit: "feat(server): Add skip-standby-site-crea..." | Re-trigger Greptile

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.10%. Comparing base (278e7a9) to head (6f573ae).
⚠️ Report is 27 commits behind head on develop.

❗ There is a different number of reports uploaded between BASE (278e7a9) and HEAD (6f573ae). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (278e7a9) HEAD (6f573ae)
dashboard 1 0
Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #6678       +/-   ##
============================================
- Coverage    62.85%   47.10%   -15.76%     
============================================
  Files          117      875      +758     
  Lines        18093    65412    +47319     
  Branches       526        0      -526     
============================================
+ Hits         11373    30812    +19439     
- Misses        6688    34600    +27912     
+ Partials        32        0       -32     
Flag Coverage Δ
dashboard ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants