For non-interactive mode, upload to GCS even if Resultstore client call fails - #45
Conversation
…ll fails Add the --no_interactive alias for --abort_with_no_creds flag
There was a problem hiding this comment.
Code Review
This pull request introduces a --no_interactive flag and modifies the results uploader to handle Resultstore API failures gracefully by falling back to GCS-only uploads. While the PR adds initial error handling, the review feedback correctly identifies that several Resultstore-related calls remain unprotected, which could still lead to script crashes and prevent GCS uploads from completing as intended.
| except Exception as e: | ||
| if args.no_interactive: | ||
| logging.warning('Resultstore API error. Continuing with GCS upload only. Error: %s', e) | ||
| rs_client = None | ||
| else: | ||
| raise |
There was a problem hiding this comment.
While catching a broad Exception ensures resilience, this block only protects the initial invocation creation. If rs_client is successfully created but a subsequent call fails (e.g., add_invocation_log at line 610 or _add_resultstore_target in the main loop), the script will crash and skip remaining GCS uploads. To fully achieve the goal of "Continuing with GCS upload only", all Resultstore-related calls should be guarded. Additionally, the initialization of rs_client at line 582 (outside this diff) should also be protected to handle cases where the client cannot be initialized at all.
| if gcs_files and rs_client: | ||
| rs_client.add_invocation_log(gcs_bucket, gcs_files[0]) |
There was a problem hiding this comment.
This call to add_invocation_log is not protected by a try-except block. If it fails, the script will crash before starting the GCS uploads for the test results, which contradicts the objective of continuing with GCS upload even if Resultstore fails.
if gcs_files and rs_client:
try:
rs_client.add_invocation_log(gcs_bucket, gcs_files[0])
except Exception as e:
if args.no_interactive:
logging.warning('Resultstore API error. Continuing with GCS upload only. Error: %s', e)
rs_client = None
else:
raise…ll fails (android#45) Add the --no_interactive alias for --abort_with_no_creds flag
Add the --no_interactive alias for --abort_with_no_creds flag