Repository navigation
Retry FPD, JUnit, and S3 publishes in the GitLab after_script - #8372
robertomonteromiguel wants to merge 3 commits into
Conversation
A stalled publish could hit the runner timeout and fail a job whose tests had passed. Co-authored-by: Cursor <cursoragent@cursor.com>
|
|
Only the S3 log upload stays exit 0 after three failed attempts. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17f0391ca9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| error: subprocess.CalledProcessError | subprocess.TimeoutExpired | None = None | ||
| for attempt in range(1, PUBLISH_ATTEMPTS + 1): | ||
| try: | ||
| run(command, env=env, timeout=PUBLISH_TIMEOUT_SECONDS) |
There was a problem hiding this comment.
In the AWS profile invoked by .base_job_onboarding_system_tests, this 30-second wall-clock timeout also applies to the recursive S3 upload, rather than only detecting a stalled transfer. The raw folder is uploaded before delete_large_log_files removes files over 50 MiB, so a healthy but slow upload can exceed 30 seconds on every attempt; because that call uses ignore_failure=True, the logs are then silently unavailable and aws_console_url remains empty. Use a larger/upload-specific timeout or an inactivity-based timeout for S3.
Useful? React with 👍 / 👎.
| run(command, env=env, timeout=PUBLISH_TIMEOUT_SECONDS) | ||
| except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as attempt_error: | ||
| error = attempt_error | ||
| print(f"⚠️ {command[0]} attempt {attempt}/{PUBLISH_ATTEMPTS} failed: {attempt_error}", flush=True) |
There was a problem hiding this comment.
When the feature-parity curl fails or times out on system-tests main, formatting attempt_error prints the complete subprocess command, including the FP_API_KEY: <secret> header assembled in publish_feature_parity. That key is fetched dynamically by read-system-tests-secret, so it is not inherently covered by GitLab's masked CI-variable handling; even a transient failure followed by a successful retry can therefore leave the credential in the job log. Log only the command name and status, or redact sensitive arguments before formatting the exception.
Useful? React with 👍 / 👎.
| error: subprocess.CalledProcessError | subprocess.TimeoutExpired | None = None | ||
| for attempt in range(1, PUBLISH_ATTEMPTS + 1): | ||
| try: | ||
| run(command, env=env, timeout=PUBLISH_TIMEOUT_SECONDS) |
There was a problem hiding this comment.
With GitLab Runner's default five-minute after_script timeout, each failed publish can now consume about 100 seconds (three 30-second attempts plus two sleeps), but none of the three jobs configures RUNNER_AFTER_SCRIPT_TIMEOUT. On a system-tests main Docker job where both FPD and JUnit are unavailable, retries alone consume roughly 200 seconds, leaving at most 100 seconds for secret reads, log collection, and the final print_logs; the runner can therefore terminate the finalizer before the promised log output and artifact preparation finish. Use one overall deadline that reserves time for later steps or configure a sufficient after-script budget in each job, as described in the GitLab Runner timeout documentation.
Useful? React with 👍 / 👎.
❌ ErrorsYour PR has failed checks. Please review the issues below and take necessary action before merging. 🚦 3 Pipeline jobs failed
ℹ️ InfoNo other issues found (see more)🧪 All tests passed Useful? React with 👍 / 👎 This comment will be updated automatically if new data arrives.🔗 Commit SHA: b5a2fb3 | Docs | View more details | Give us feedback! |
Motivation
A stalled publish in the GitLab after_script can reach the runner timeout and fail a job whose tests passed.
Changes
Retry the feature-parity curl, the JUnit upload, and the S3 log upload three times. Each attempt stops after 30 seconds.
If the S3 upload still fails, log a warning and continue with exit code 0. If the feature-parity curl or the JUnit upload still fails, the after_script exits 1.
Test plan
pytest tests/test_the_test/test_gitlab_finalize_job.py --scenario TEST_THE_TEST