Skip to content

For non-interactive mode, upload to GCS even if Resultstore client call fails #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
xianyuanjia merged 1 commit into from
Apr 21, 2026
Merged

For non-interactive mode, upload to GCS even if Resultstore client call fails #45

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions src/results_uploader/results_uploader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ def _parse_args(argv: list[str] | None) -> argparse.Namespace:
'--duration', type=int, help=argparse.SUPPRESS
)
parser.add_argument(
'--abort_if_no_creds', action='store_true', help=argparse.SUPPRESS
'--no_interactive', '--abort_if_no_creds', action='store_true',
help=argparse.SUPPRESS
)
return parser.parse_args(args=argv or sys.argv[1:])

Expand Down Expand Up @@ -488,7 +489,7 @@ def _create_resultstore_invocation(


def _add_resultstore_target(
client: resultstore_client.ResultstoreClient,
client: resultstore_client.ResultstoreClient | None,
gcs_bucket: str,
gcs_dir: str,
file_paths: list[str],
Expand All @@ -497,6 +498,8 @@ def _add_resultstore_target(
assign_undeclared_outputs: bool = False,
) -> None:
"""Calls the Resultstore Upload API to create and populate a new target."""
if not client:
return
client.create_target(target_id)
client.create_configured_target()
client.create_action(gcs_bucket, gcs_dir, file_paths, assign_undeclared_outputs)
Expand All @@ -507,11 +510,13 @@ def _add_resultstore_target(


def _finalize_resultstore_invocation(
client: resultstore_client.ResultstoreClient,
client: resultstore_client.ResultstoreClient | None,
status: _Status,
labels: list[str],
):
"""Updates the final status of the invocation and completes the upload."""
if not client:
return
client.merge_invocation(status, labels)
client.finalize_invocation()

Expand Down Expand Up @@ -557,9 +562,9 @@ def main(argv: list[str] | None = None) -> None:
try:
creds, project_id = google.auth.default()
except google.auth.exceptions.DefaultCredentialsError:
if args.abort_if_no_creds:
if args.no_interactive:
logging.error(
'No local credentials found (and abort_if_no_creds==True); '
'No local credentials found (and no_interactive==True); '
'aborting upload. Please run gcloud_setup.py to login first.'
)
exit(1)
Expand All @@ -586,15 +591,22 @@ def main(argv: list[str] | None = None) -> None:
test_timing = None
if args.start_time:
test_timing = resultstore_client.Timing(args.start_time, args.duration)
_create_resultstore_invocation(rs_client, test_timing)
try:
_create_resultstore_invocation(rs_client, test_timing)
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
Comment on lines +596 to +601

@gemini-code-assist gemini-code-assist Bot Apr 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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.


# Upload CTS console log as invocation log
if cts_console_log_dir:
gcs_files = _upload_dir_to_gcs(
cts_console_log_dir, gcs_bucket, gcs_base_dir.as_posix(),
args.gcs_upload_timeout
)
if gcs_files:
if gcs_files and rs_client:
rs_client.add_invocation_log(gcs_bucket, gcs_files[0])
Comment on lines +609 to 610

@gemini-code-assist gemini-code-assist Bot Apr 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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


target_statuses = []
Expand Down Expand Up @@ -652,7 +664,8 @@ def main(argv: list[str] | None = None) -> None:
)
target_statuses.append(test_result_info.status)
finally:
logging.info('Generating final Resultstore link...')
if rs_client:
logging.info('Generating final Resultstore link...')
invocation_status = _aggregate_subtest_results(target_statuses)
labels = args.label
if args.label_on_pass_only:
Expand Down
Loading