Skip to content

Commit fb725e9

Browse files
jonsimantovgoogle-labs-jules[bot]
jonsimantov
and
google-labs-jules[bot]
authored
Fix build status report (#1764)
* Fix: Improve artifact fetching and error handling in build status script - Implemented exponential backoff and increased retries for API requests in `firebase_github.py`. - Added specific handling for 410 errors when downloading artifacts, preventing retries for expired artifacts. - Updated `report_build_status.py` to prioritize newer artifacts and added timeouts for downloads. - Ensured the script falls back to reading GitHub logs if artifacts cannot be downloaded. - Addressed missing dependencies: `python-dateutil`, `progress`, and `attrs`. - Corrected the `--verbosity` flag usage. * Feat: Configure separate artifact retention for logs and binaries Introduced two new environment variables in `integration_tests.yml`: - `logArtifactRetentionDays` set to 90 days for log artifacts. - `binaryArtifactRetentionDays` set to 7 days for binary artifacts (test apps, videos, etc.). Updated all `actions/upload-artifact` steps to use the corresponding environment variable for `retention-days` based on the type of artifact being uploaded. (Previous commit used snake_case; this commit corrects to camelCase for consistency with existing `artifactRetentionDays` variable.) * Chore: Address review comments in firebase_github.py Removed comments related to `allowed_methods` as they were deemed unnecessary. * Chore: Address review comments in firebase_github.py Removed comments related to `allowed_methods` as they were deemed unnecessary. --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
1 parent 648848c commit fb725e9

File tree

3 files changed

+101
-37
lines changed

3 files changed

+101
-37
lines changed

‎.github/workflows/integration_tests.yml‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ env:
5050
triggerLabelQuick: "tests-requested: quick"
5151
pythonVersion: '3.8'
5252
xcodeVersion: '16.2'
53-
artifactRetentionDays: 2
53+
logArtifactRetentionDays: 90
54+
binaryArtifactRetentionDays: 7
5455
GITHUB_TOKEN: ${{ github.token }}
5556

5657
jobs:
@@ -376,14 +377,14 @@ jobs:
376377
with:
377378
name: testapps-desktop-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.ssl_variant }}
378379
path: testapps-desktop-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.ssl_variant }}
379-
retention-days: ${{ env.artifactRetentionDays }}
380+
retention-days: ${{ env.binaryArtifactRetentionDays }}
380381
- name: Upload Desktop build results artifact
381382
uses: actions/upload-artifact@v4
382383
if: ${{ !cancelled() }}
383384
with:
384385
name: log-artifact-build-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.ssl_variant }}
385386
path: build-results-desktop-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.ssl_variant }}*
386-
retention-days: ${{ env.artifactRetentionDays }}
387+
retention-days: ${{ env.logArtifactRetentionDays }}
387388
- name: Download log artifacts
388389
if: ${{ needs.check_and_prepare.outputs.pr_number && failure() && !cancelled() }}
389390
uses: actions/download-artifact@v4
@@ -496,14 +497,14 @@ jobs:
496497
with:
497498
name: testapps-android-${{ matrix.os }}
498499
path: testapps-android-${{ matrix.os }}
499-
retention-days: ${{ env.artifactRetentionDays }}
500+
retention-days: ${{ env.binaryArtifactRetentionDays }}
500501
- name: Upload Android build results artifact
501502
uses: actions/upload-artifact@v4
502503
if: ${{ !cancelled() }}
503504
with:
504505
name: log-artifact-build-android-${{ matrix.os }}
505506
path: build-results-android-${{ matrix.os }}*
506-
retention-days: ${{ env.artifactRetentionDays }}
507+
retention-days: ${{ env.logArtifactRetentionDays }}
507508
- name: Download log artifacts
508509
if: ${{ needs.check_and_prepare.outputs.pr_number && failure() && !cancelled() }}
509510
uses: actions/download-artifact@v4
@@ -604,14 +605,14 @@ jobs:
604605
with:
605606
name: testapps-ios-${{ matrix.os }}
606607
path: testapps-ios-${{ matrix.os }}
607-
retention-days: ${{ env.artifactRetentionDays }}
608+
retention-days: ${{ env.binaryArtifactRetentionDays }}
608609
- name: Upload iOS build results artifact
609610
uses: actions/upload-artifact@v4
610611
if: ${{ !cancelled() }}
611612
with:
612613
name: log-artifact-build-ios-${{ matrix.os }}
613614
path: build-results-ios-${{ matrix.os }}*
614-
retention-days: ${{ env.artifactRetentionDays }}
615+
retention-days: ${{ env.logArtifactRetentionDays }}
615616
- name: Download log artifacts
616617
if: ${{ needs.check_and_prepare.outputs.pr_number && failure() && !cancelled() }}
617618
uses: actions/download-artifact@v4
@@ -711,14 +712,14 @@ jobs:
711712
with:
712713
name: testapps-tvos-${{ matrix.os }}
713714
path: testapps-tvos-${{ matrix.os }}
714-
retention-days: ${{ env.artifactRetentionDays }}
715+
retention-days: ${{ env.binaryArtifactRetentionDays }}
715716
- name: Upload tvOS build results artifact
716717
uses: actions/upload-artifact@v4
717718
if: ${{ !cancelled() }}
718719
with:
719720
name: log-artifact-build-tvos-${{ matrix.os }}
720721
path: build-results-tvos-${{ matrix.os }}*
721-
retention-days: ${{ env.artifactRetentionDays }}
722+
retention-days: ${{ env.logArtifactRetentionDays }}
722723
- name: Download log artifacts
723724
if: ${{ needs.check_and_prepare.outputs.pr_number && failure() && !cancelled() }}
724725
uses: actions/download-artifact@v4
@@ -848,7 +849,7 @@ jobs:
848849
with:
849850
name: log-artifact-test-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.ssl_variant }}
850851
path: testapps/test-results-desktop-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.ssl_variant }}*
851-
retention-days: ${{ env.artifactRetentionDays }}
852+
retention-days: ${{ env.logArtifactRetentionDays }}
852853
- name: Download log artifacts
853854
if: ${{ needs.check_and_prepare.outputs.pr_number && failure() && !cancelled() }}
854855
uses: actions/download-artifact@v4
@@ -963,21 +964,21 @@ jobs:
963964
with:
964965
name: log-artifact-test-android-${{ matrix.build_os }}-${{ matrix.android_device }}-${{ matrix.test_type }}
965966
path: testapps/test-results-android-${{ matrix.build_os }}-${{ matrix.android_device }}-${{ matrix.test_type }}*
966-
retention-days: ${{ env.artifactRetentionDays }}
967+
retention-days: ${{ env.logArtifactRetentionDays }}
967968
- name: Upload Android test video artifact
968969
if: ${{ steps.device-info.outputs.device_type == 'virtual' && !cancelled() }}
969970
uses: actions/upload-artifact@v4
970971
with:
971972
name: mobile-simulator-test-video-artifact-${{ matrix.build_os }}-${{ matrix.android_device }}-${{ matrix.test_type }}
972973
path: testapps/video-*-android-${{ matrix.build_os }}-${{ matrix.android_device }}-${{ matrix.test_type }}.mp4
973-
retention-days: ${{ env.artifactRetentionDays }}
974+
retention-days: ${{ env.binaryArtifactRetentionDays }}
974975
- name: Upload Android test logcat artifact
975976
if: ${{ steps.device-info.outputs.device_type == 'virtual' && !cancelled() }}
976977
uses: actions/upload-artifact@v4
977978
with:
978979
name: mobile-simulator-test-logcat-artifact-${{ matrix.build_os }}-${{ matrix.android_device }}-${{ matrix.test_type }}
979980
path: testapps/logcat-*-android-${{ matrix.build_os }}-${{ matrix.android_device }}-${{ matrix.test_type }}.txt
980-
retention-days: ${{ env.artifactRetentionDays }}
981+
retention-days: ${{ env.logArtifactRetentionDays }}
981982
- name: Download log artifacts
982983
if: ${{ needs.check_and_prepare.outputs.pr_number && failure() && !cancelled() }}
983984
uses: actions/download-artifact@v4
@@ -1149,14 +1150,14 @@ jobs:
11491150
with:
11501151
name: log-artifact-test-ios-${{ matrix.build_os }}-${{ matrix.ios_device }}-${{ matrix.test_type }}
11511152
path: testapps/test-results-ios-${{ matrix.build_os }}-${{ matrix.ios_device }}-${{ matrix.test_type }}*
1152-
retention-days: ${{ env.artifactRetentionDays }}
1153+
retention-days: ${{ env.logArtifactRetentionDays }}
11531154
- name: Upload iOS test video artifact
11541155
if: ${{ steps.device-info.outputs.device_type == 'virtual' && !cancelled() }}
11551156
uses: actions/upload-artifact@v4
11561157
with:
11571158
name: mobile-simulator-test-video-artifact-ios-${{ matrix.build_os }}-${{ matrix.ios_device }}-${{ matrix.test_type }}
11581159
path: testapps/video-*-ios-${{ matrix.build_os }}-${{ matrix.ios_device }}-${{ matrix.test_type }}.mp4
1159-
retention-days: ${{ env.artifactRetentionDays }}
1160+
retention-days: ${{ env.binaryArtifactRetentionDays }}
11601161
- name: Download log artifacts
11611162
if: ${{ needs.check_and_prepare.outputs.pr_number && failure() && !cancelled() }}
11621163
uses: actions/download-artifact@v4
@@ -1289,14 +1290,14 @@ jobs:
12891290
with:
12901291
name: log-artifact-test-tvos-${{ matrix.build_os }}-${{ matrix.tvos_device }}
12911292
path: testapps/test-results-tvos-${{ matrix.build_os }}-${{ matrix.tvos_device }}*
1292-
retention-days: ${{ env.artifactRetentionDays }}
1293+
retention-days: ${{ env.logArtifactRetentionDays }}
12931294
- name: Upload tvOS test video artifact
12941295
if: ${{ !cancelled() }}
12951296
uses: actions/upload-artifact@v4
12961297
with:
12971298
name: mobile-simulator-test-video-artifact-tvos-${{ matrix.build_os }}-${{ matrix.tvos_device }}
12981299
path: testapps/video-*-tvos-${{ matrix.build_os }}-${{ matrix.tvos_device }}.mp4
1299-
retention-days: ${{ env.artifactRetentionDays }}
1300+
retention-days: ${{ env.binaryArtifactRetentionDays }}
13001301
- name: Download log artifacts
13011302
if: ${{ needs.check_and_prepare.outputs.pr_number && failure() && !cancelled() }}
13021303
uses: actions/download-artifact@v4

‎scripts/gha/firebase_github.py‎

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,15 @@ def set_repo_url(repo):
5757

5858
def requests_retry_session(retries=RETRIES,
5959
backoff_factor=BACKOFF,
60-
status_forcelist=RETRY_STATUS):
60+
status_forcelist=RETRY_STATUS,
61+
allowed_methods=frozenset(['GET', 'POST', 'PUT', 'DELETE', 'PATCH'])):
6162
session = requests.Session()
6263
retry = Retry(total=retries,
6364
read=retries,
6465
connect=retries,
6566
backoff_factor=backoff_factor,
66-
status_forcelist=status_forcelist)
67+
status_forcelist=status_forcelist,
68+
allowed_methods=allowed_methods)
6769
adapter = HTTPAdapter(max_retries=retry)
6870
session.mount('http://', adapter)
6971
session.mount('https://', adapter)
@@ -183,14 +185,36 @@ def download_artifact(token, artifact_id, output_path=None):
183185
"""https://docs.github.com/en/rest/reference/actions#download-an-artifact"""
184186
url = f'{GITHUB_API_URL}/actions/artifacts/{artifact_id}/zip'
185187
headers = {'Accept': 'application/vnd.github.v3+json', 'Authorization': f'token {token}'}
186-
with requests_retry_session().get(url, headers=headers, stream=True, timeout=TIMEOUT_LONG) as response:
187-
logging.info("download_artifact: %s response: %s", url, response)
188-
if output_path:
189-
with open(output_path, 'wb') as file:
190-
shutil.copyfileobj(response.raw, file)
191-
elif response.status_code == 200:
192-
return response.content
193-
return None
188+
# Custom retry for artifact download due to potential for 410 errors (artifact expired)
189+
# which shouldn't be retried indefinitely like other server errors.
190+
artifact_retry = Retry(total=5, # Increased retries
191+
read=5,
192+
connect=5,
193+
backoff_factor=1, # More aggressive backoff for artifacts
194+
status_forcelist=(500, 502, 503, 504), # Only retry on these server errors
195+
allowed_methods=frozenset(['GET']))
196+
session = requests.Session()
197+
adapter = HTTPAdapter(max_retries=artifact_retry)
198+
session.mount('https://', adapter)
199+
200+
try:
201+
with session.get(url, headers=headers, stream=True, timeout=TIMEOUT_LONG) as response:
202+
logging.info("download_artifact: %s response: %s", url, response)
203+
response.raise_for_status() # Raise an exception for bad status codes
204+
if output_path:
205+
with open(output_path, 'wb') as file:
206+
shutil.copyfileobj(response.raw, file)
207+
return True # Indicate success
208+
else:
209+
return response.content
210+
except requests.exceptions.HTTPError as e:
211+
logging.error(f"HTTP error downloading artifact {artifact_id}: {e.response.status_code} - {e.response.reason}")
212+
if e.response.status_code == 410:
213+
logging.warning(f"Artifact {artifact_id} has expired and cannot be downloaded.")
214+
return None # Indicate failure
215+
except requests.exceptions.RequestException as e:
216+
logging.error(f"Error downloading artifact {artifact_id}: {e}")
217+
return None # Indicate failure
194218

195219

196220
def dismiss_review(token, pull_number, review_id, message):

‎scripts/gha/report_build_status.py‎

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -471,24 +471,63 @@ def main(argv):
471471
found_artifacts = False
472472
# There are possibly multiple artifacts, so iterate through all of them,
473473
# and extract the relevant ones into a temp folder, and then summarize them all.
474+
# Prioritize artifacts by date, older ones might be expired.
475+
sorted_artifacts = sorted(artifacts, key=lambda art: dateutil.parser.parse(art['created_at']), reverse=True)
476+
474477
with tempfile.TemporaryDirectory() as tmpdir:
475-
for a in artifacts:
478+
for a in sorted_artifacts: # Iterate over sorted artifacts
476479
if 'log-artifact' in a['name']:
477-
print("Checking this artifact:", a['name'], "\n")
478-
artifact_contents = firebase_github.download_artifact(FLAGS.token, a['id'])
479-
if artifact_contents:
480-
found_artifacts = True
481-
artifact_data = io.BytesIO(artifact_contents)
482-
artifact_zip = zipfile.ZipFile(artifact_data)
483-
artifact_zip.extractall(path=tmpdir)
480+
logging.debug("Attempting to download artifact: %s (ID: %s, Created: %s)", a['name'], a['id'], a['created_at'])
481+
# Pass tmpdir to download_artifact to save directly
482+
artifact_downloaded_path = os.path.join(tmpdir, f"{a['id']}.zip")
483+
# Attempt to download the artifact with a timeout
484+
download_success = False # Initialize download_success
485+
try:
486+
# download_artifact now returns True on success, None on failure.
487+
if firebase_github.download_artifact(FLAGS.token, a['id'], output_path=artifact_downloaded_path):
488+
download_success = True
489+
except requests.exceptions.Timeout:
490+
logging.warning(f"Timeout while trying to download artifact: {a['name']} (ID: {a['id']})")
491+
# download_success remains False
492+
493+
if download_success and os.path.exists(artifact_downloaded_path):
494+
try:
495+
with open(artifact_downloaded_path, "rb") as f:
496+
artifact_contents = f.read()
497+
if artifact_contents: # Ensure content was read
498+
found_artifacts = True
499+
artifact_data = io.BytesIO(artifact_contents)
500+
with zipfile.ZipFile(artifact_data) as artifact_zip: # Use with statement for ZipFile
501+
artifact_zip.extractall(path=tmpdir)
502+
logging.info("Successfully downloaded and extracted artifact: %s", a['name'])
503+
else:
504+
logging.warning("Artifact %s (ID: %s) was downloaded but is empty.", a['name'], a['id'])
505+
except zipfile.BadZipFile:
506+
logging.error("Failed to open zip file for artifact %s (ID: %s). It might be corrupted or not a zip file.", a['name'], a['id'])
507+
except Exception as e:
508+
logging.error("An error occurred during artifact processing %s (ID: %s): %s", a['name'], a['id'], e)
509+
finally:
510+
# Clean up the downloaded zip file whether it was processed successfully or not
511+
if os.path.exists(artifact_downloaded_path):
512+
os.remove(artifact_downloaded_path)
513+
elif not download_success : # Covers False or None from download_artifact
514+
# Logging for non-timeout failures is now primarily handled within download_artifact
515+
# We only log a general failure here if it wasn't a timeout (already logged)
516+
# and download_artifact indicated failure (returned None).
517+
# This avoids double logging for specific HTTP errors like 410.
518+
pass # Most specific logging is now in firebase_github.py
519+
484520
if found_artifacts:
485521
(success, results) = summarize_test_results.summarize_logs(tmpdir, False, False, True)
486-
print("Results:", success, " ", results, "\n")
522+
logging.info("Summarized logs results - Success: %s, Results (first 100 chars): %.100s", success, results)
487523
run['log_success'] = success
488524
run['log_results'] = results
525+
else:
526+
logging.warning("No artifacts could be successfully downloaded and processed for run %s on day %s.", run['id'], day)
527+
489528

490529
if not found_artifacts:
491-
# Artifacts expire after some time, so if they are gone, we need
530+
# Artifacts expire after some time, or download failed, so if they are gone, we need
492531
# to read the GitHub logs instead. This is much slower, so we
493532
# prefer to read artifacts instead whenever possible.
494533
logging.info("Reading github logs for run %s instead", run['id'])

0 commit comments

Comments
 (0)