Add timeout and retry to combine_restore kubectl cp

[imnasnainaec]

Fixes

Fixes #4322

Problem

maintenance/scripts/combine_restore.py copies backend files into the backend pod with a per-project kubectl cp loop (and copies the DB dump the same way). kubectl cp streams a tar over the exec channel and can stall intermittently. Because maint_utils.run_cmd() called subprocess.run(...) with no timeout, a stalled copy hung forever with no error and no output. Since this is The Combine's disaster-recovery restore, a silent infinite hang is a serious reliability problem.

Changes

• maint_utils.py: add an optional timeout parameter to run_cmd(), passed through to subprocess.run(...). When set, a stalled command is killed and subprocess.TimeoutExpired is raised for the caller to handle. Default None preserves existing behavior.
• combine_app.py: thread timeout through CombineApp.exec() and CombineApp.kubectl() (also adding check_results to kubectl()).
• combine_restore.py: wrap each kubectl cp (the database dump and every backend-file item) in a new kubectl_cp() helper that:
  • bounds each attempt with a timeout so a stalled stream is killed;
  • retries a few times so a transient stall is recovered instead of aborting the whole restore;
  • on repeated failure, logs the failing item (project) and exits non-zero so it surfaces as an error rather than a silent hang.
• The timeout and retry count are configurable via the kubectl_cp_timeout (default 300s) and kubectl_cp_retries (default 3) environment variables.

I kept the existing per-item kubectl cp loop rather than switching to a single tar-pipe (listed as optional in the issue) to keep the change focused; the timeout/retry guard applies either way.

Testing

• tox equivalents pass locally on the changed files: isort --check, black --check, flake8, and mypy --platform linux maintenance/scripts (strict) — all clean.
• Exercised the new logic directly:
  • run_cmd(..., timeout=1) on a 30s sleep raises TimeoutExpired (no longer hangs) and still returns output for fast commands.
  • kubectl_cp() succeeds without retry on a clean copy (and passes check_results=False + timeout), recovers from transient stalls (timeout → timeout → success), and exits non-zero after exhausting retries on repeated failure.

🤖 Generated with Claude Code

Devin review: https://app.devin.ai/review/sillsdev/TheCombine/pull/4323

---

This change is 

Summary by CodeRabbit

• Bug Fixes

  • Improved backup and restore reliability by retrying file copy operations that can stall or fail temporarily.
  • Added timeouts to long-running copy and command operations so they fail more predictably instead of hanging indefinitely.
  • Backup and restore flows now use clearer copy-step labeling for better troubleshooting.

• Chores

  • Added configurable environment-based settings for copy timeouts and retry attempts.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cdd1de7f-4803-404f-aebf-3f9c12d8a39e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-4322-cp-timeout

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.88%. Comparing base (e2b613d) to head (2062d90).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4323      +/-   ##
==========================================
- Coverage   75.96%   66.88%   -9.08%     
==========================================
  Files         305      248      -57     
  Lines       11384     6303    -5081     
  Branches     1411      794     -617     
==========================================
- Hits         8648     4216    -4432     
+ Misses       2332     1826     -506     
+ Partials      404      261     -143     

Flag	Coverage Δ
backend	?
frontend	66.88% <ø> (ø)

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.

[coderabbitai]

🧹 Nitpick comments (1)

maintenance/scripts/combine_app.py (1)

100-140: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Optional: add a short backoff between retries.

Attempts currently retry immediately after a non-zero return code. A brief sleep between attempts would avoid hammering a transient failure that isn't a timeout. Timeout attempts already have inherent delay, so this mainly helps the fast-fail path.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@maintenance/scripts/combine_app.py` around lines 100 - 140, The retry loop in
cp_with_retry currently retries immediately after a non-zero kubectl cp failure,
which can rapidly hammer a transient fast-fail path. Add a short sleep between
failed attempts in cp_with_retry (for the returncode != 0 case, and optionally
after timeouts if desired) before the next iteration, using the existing
attempt/attempts logging to keep behavior clear. Keep the change localized to
cp_with_retry and preserve the final abort behavior after all attempts are
exhausted.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@maintenance/scripts/combine_app.py`:
- Around line 100-140: The retry loop in cp_with_retry currently retries
immediately after a non-zero kubectl cp failure, which can rapidly hammer a
transient fast-fail path. Add a short sleep between failed attempts in
cp_with_retry (for the returncode != 0 case, and optionally after timeouts if
desired) before the next iteration, using the existing attempt/attempts logging
to keep behavior clear. Keep the change localized to cp_with_retry and preserve
the final abort behavior after all attempts are exhausted.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 729281c5-f5f1-4e34-8f56-0b6bcceee7db

📥 Commits

Reviewing files that changed from the base of the PR and between e2b613d and adae68b.

📒 Files selected for processing (4)

• maintenance/scripts/combine_app.py
• maintenance/scripts/combine_backup.py
• maintenance/scripts/combine_restore.py
• maintenance/scripts/maint_utils.py