Skip to content
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.

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

Upload all logs in BEP even with minimal upload #17110

Closed
wants to merge 1 commit into from

Conversation

exoson
Copy link
Contributor

@exoson exoson commented Jan 2, 2023

When using --experimental_remote_build_event_upload=minimal, build action logs won't get uploaded. These logs are still very useful as when a build action fails, one would need to inspect them to figure out what has gone wrong. Forcing uploading stdout and stderr with this change.

@exoson exoson requested a review from a team as a code owner January 2, 2023 14:32
@sgowroji sgowroji added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Jan 3, 2023
Copy link
Member

@coeuvre coeuvre left a comment

Choose a reason for hiding this comment

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

Do we always want to upload action logs or only upload when they failed?

Please add a test case for this similar to test_upload_minimal_upload_testlogs.

@coeuvre coeuvre added awaiting-user-response Awaiting a response from the author and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jan 4, 2023
When using --experimental_remote_build_event_upload=minimal, build
action logs won't get uploaded. These logs are still very useful as when
a build action fails, one would need to inspect them to figure out what
has gone wrong. Forcing uploading stdout and stderr with this change.
@exoson
Copy link
Contributor Author

exoson commented Jan 5, 2023

Seems that at least currently the build event file won't reference the build logs unless the action had failed so the logs will only be uploaded for failing actions.

@coeuvre coeuvre added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-user-response Awaiting a response from the author labels Jan 5, 2023
@brentleyjones
Copy link
Contributor

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Jan 5, 2023
@copybara-service copybara-service bot closed this in 17b8e44 Jan 9, 2023
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jan 10, 2023
@ShreeM01
Copy link
Contributor

@bazel-io fork 6.1.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Jan 19, 2023
ShreeM01 added a commit that referenced this pull request Jan 24, 2023
When using --experimental_remote_build_event_upload=minimal, build action logs won't get uploaded. These logs are still very useful as when a build action fails, one would need to inspect them to figure out what has gone wrong. Forcing uploading stdout and stderr with this change.

Closes #17110.

PiperOrigin-RevId: 500723770
Change-Id: I5e2edb6356b55549cd160379273af9a1580945d3

Co-authored-by: Emil Kattainen <emilkattainen@gmail.com>
hvadehra pushed a commit that referenced this pull request Feb 14, 2023
When using --experimental_remote_build_event_upload=minimal, build action logs won't get uploaded. These logs are still very useful as when a build action fails, one would need to inspect them to figure out what has gone wrong. Forcing uploading stdout and stderr with this change.

Closes #17110.

PiperOrigin-RevId: 500723770
Change-Id: I5e2edb6356b55549cd160379273af9a1580945d3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants