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

Propagate the error message when a credential helper fails. #16012

Closed
wants to merge 1 commit into from

Conversation

tjgq
Copy link
Contributor

@tjgq tjgq commented Aug 1, 2022

No description provided.

@tjgq tjgq requested a review from coeuvre August 1, 2022 14:59
@tjgq tjgq requested a review from a team as a code owner August 1, 2022 14:59
@tjgq
Copy link
Contributor Author

tjgq commented Aug 1, 2022

@Yannic

package com.google.devtools.build.lib.authandtls.credentialhelper;

/** An {@link Exception} thrown while invoking a credential helper. */
public class CredentialHelperException extends RuntimeException {
Copy link
Member

@coeuvre coeuvre Aug 1, 2022

Choose a reason for hiding this comment

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

Can this extend IOException?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I've also fixed the test assertions.

Copy link
Contributor

@Yannic Yannic left a comment

Choose a reason for hiding this comment

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

Thanks! Do we need to (a) cherry-pick to 5.3 and (b) also change the error for BES upload?

@tjgq
Copy link
Contributor Author

tjgq commented Aug 1, 2022

Thanks! Do we need to (a) cherry-pick to 5.3 and (b) also change the error for BES upload?

BES already includes the cause in the error message: https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/buildeventservice/client/BuildEventServiceGrpcClient.java;l=183;drc=f141e149ac3210e417b31aec21afed267cdbb092

I'm not sure whether we're still accepting cherry-picks for 5.3, but I'll ask.

@tjgq
Copy link
Contributor Author

tjgq commented Aug 2, 2022

@bazel-io fork 5.3.0

@copybara-service copybara-service bot closed this in 24c16fc Aug 2, 2022
ShreeM01 pushed a commit that referenced this pull request Aug 2, 2022
Closes #16012.

PiperOrigin-RevId: 464732834
Change-Id: If51ce914098ff17ffe23fa4614947e7f4a5088dc
@ShreeM01
Copy link
Contributor

ShreeM01 commented Aug 2, 2022

Hey @tjgq we are trying to [cherry-pick] (#16030) the changes to release 5.3.0. But pre-submits checks are failing due to missing dependencies. So could you please help us in cherry-picking this changes to release 5.3.0? Thanks!

ShreeM01 added a commit that referenced this pull request Aug 3, 2022
Closes #16012.

PiperOrigin-RevId: 464732834
Change-Id: If51ce914098ff17ffe23fa4614947e7f4a5088dc

Co-authored-by: Tiago Quelhas <tjgq@google.com>
ShreeM01 added a commit that referenced this pull request Aug 9, 2022
* Updated Codeowners file (#16032)

* Propagate the error message when a credential helper fails. (#16030)

Closes #16012.

PiperOrigin-RevId: 464732834
Change-Id: If51ce914098ff17ffe23fa4614947e7f4a5088dc

Co-authored-by: Tiago Quelhas <tjgq@google.com>

* Migrate legacy desugar wrapper to new rlocation() (#16025)

RELNOTES: Fix for desugaring failure on Bazel+Android+Windows build scenario.
PiperOrigin-RevId: 464654335
Change-Id: I845093b134dc68ae541603008fe7fee701c7451c

Co-authored-by: Gowroji Sunil <48122892+sgowroji@users.noreply.github.com>

* Correctly report errors thrown by CommandLinePathFactory#create. (#16036)

This method is called while initializing the remote module [1]. Any exceptions
not derived from java.io.IOException cause a silent server crash.

[1] https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java;l=436;drc=f3211f00ae08746b5794ab01d404c32b43146aba

Closes #16022.

PiperOrigin-RevId: 464997904
Change-Id: I87fd5eaeb562d849bb830d68f1bfbbf06f6e0ea9

Co-authored-by: Tiago Quelhas <tjgq@google.com>

* Fix an issue that `incompatible_remote_build_event_upload_respect_no_… (#16045)

* Fix an issue that `incompatible_remote_build_event_upload_respect_no_…

…cache` doesn't work with BwtB.

The root cause is we use `Path` as key to check which files are omitted, but it also compares the underlying filesystem. When BwtB is enabled, the filesystem for the file is different so we failed to mark the file as omitted.

This change fixes that by using `PathFragment` as key.

Fixes #15527.

Closes #16023.

PiperOrigin-RevId: 465284879
Change-Id: I49bbd7a6055e0f234b4ac86a224886bd85797f71

* Update ByteStreamBuildEventArtifactUploader changes

Removing the import as it is throwing the error

Co-authored-by: Chi Wang <chiwang@google.com>

Co-authored-by: Tiago Quelhas <tjgq@google.com>
Co-authored-by: Ted <tedx@google.com>
Co-authored-by: Gowroji Sunil <48122892+sgowroji@users.noreply.github.com>
Co-authored-by: Chi Wang <chiwang@google.com>
@ShreeM01 ShreeM01 removed the awaiting-review PR is awaiting review from an assigned reviewer label Sep 15, 2022
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.

4 participants