Skip to content

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

Merged
xianyuanjia merged 1 commit into
android:mainfrom
xianyuanjia:no_interactive
Apr 21, 2026
Merged

For non-interactive mode, upload to GCS even if Resultstore client call fails#45
xianyuanjia merged 1 commit into
android:mainfrom
xianyuanjia:no_interactive

Conversation

@xianyuanjia

Copy link
Copy Markdown
Collaborator

Add the --no_interactive alias for --abort_with_no_creds flag

…ll fails

Add the --no_interactive alias for --abort_with_no_creds flag
@xianyuanjia
xianyuanjia requested a review from mhaoli April 21, 2026 01:18

@gemini-code-assist gemini-code-assist Bot left a comment

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.

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.

Comment on lines +596 to +601
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

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.

Comment on lines +609 to 610
if gcs_files and rs_client:
rs_client.add_invocation_log(gcs_bucket, gcs_files[0])

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

@xianyuanjia
xianyuanjia merged commit 2d3854e into android:main Apr 21, 2026
2 checks passed
@xianyuanjia
xianyuanjia deleted the no_interactive branch April 21, 2026 18:21
xianyuanjia added a commit to xianyuanjia/mobly-android-partner-tools that referenced this pull request Apr 28, 2026
…ll fails (android#45)

Add the --no_interactive alias for --abort_with_no_creds flag
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants