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.

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

Jump to bottom

Add netrc support to --bes_backend #15930

Closed
wants to merge 7 commits into from
Closed

Add netrc support to --bes_backend #15930

wants to merge 7 commits into from

Conversation

Yannic
Copy link
Contributor

@Yannic Yannic commented Jul 20, 2022

Progress on #15856

Fixes #15928

@Yannic
Copy link
Contributor Author

Yannic commented Jul 20, 2022

@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 Jul 20, 2022
@Yannic Yannic marked this pull request as ready for review July 20, 2022 12:48
@Yannic Yannic requested a review from a team as a code owner July 20, 2022 12:48
@Yannic
Copy link
Contributor Author

Yannic commented Jul 20, 2022

@tjgq PTAL

tjgq
tjgq requested changes Jul 20, 2022
View reviewed changes
src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java Outdated Show resolved Hide resolved
src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java Outdated Show resolved Hide resolved
src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java Outdated
@@ -191,7 +197,21 @@ private static NettyChannelBuilder newNettyChannelBuilder(String targetUrl, Stri
* @throws IOException in case the call credentials can't be constructed.
*/
@Nullable
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy the comment from newCallCredentials here, to make it clear where we attempt to retrieve credentials from.

Copy link
Contributor Author

@Yannic Yannic Jul 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Found a simple way to delete the method instead.

src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java Outdated Show resolved Hide resolved
@sgowroji
Copy link
Member

@bazel-io fork 5.3.0

@bazel-io bazel-io mentioned this pull request Jul 20, 2022
Closed
@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 Jul 20, 2022
tjgq
tjgq requested changes Jul 20, 2022
View reviewed changes
Copy link
Contributor

@tjgq tjgq left a comment

Choose a reason for hiding this comment

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

Looks like //src/test/java/com/google/devtools/build/lib/buildeventservice:BazelBuildEventServiceModuleTest needs fixing?

@sgowroji sgowroji added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-user-response Awaiting a response from the author labels Jul 21, 2022
@Yannic
Copy link
Contributor Author

Yannic commented Jul 21, 2022

Fixed, PTAL

@tjgq
Copy link
Contributor

tjgq commented Jul 21, 2022

FYI, I'll have to remove a couple of @VisibleForTesting annotations when importing, because of Google-internal callers that aren't part of the open source release.

@sgowroji sgowroji 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 Jul 21, 2022
Yannic added a commit to EngFlow/bazel that referenced this pull request Jul 21, 2022
@Yannic
Move newCredentialHelperProvider into GoogleAuthUtils
e42252c 
Follow-up on bazelbuild#15930 due to
bazelbuild#15906

Progress on bazelbuild#15856
@Yannic Yannic mentioned this pull request Jul 21, 2022
Closed
copybara-service bot pushed a commit that referenced this pull request Jul 22, 2022
@Yannic @copybara-github
Move newCredentialHelperProvider into GoogleAuthUtils
446b1a2 
Follow-up on #15930 due to
#15906

Progress on #15856

Closes #15941.

PiperOrigin-RevId: 462580799
Change-Id: Ibac79ed68a0c87a4f34eb9d0729abb1552b44519
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jul 22, 2022
sgowroji pushed a commit to sgowroji/bazel that referenced this pull request Jul 22, 2022
@Yannic
Move newCredentialHelperProvider into GoogleAuthUtils
5e7351b 
Follow-up on bazelbuild#15930 due to
bazelbuild#15906

Progress on bazelbuild#15856

Closes bazelbuild#15941.

PiperOrigin-RevId: 462580799
Change-Id: Ibac79ed68a0c87a4f34eb9d0729abb1552b44519
@sgowroji sgowroji mentioned this pull request Jul 22, 2022
Closed
ckolli5 pushed a commit to ckolli5/bazel that referenced this pull request Jul 22, 2022
@Yannic @ckolli5
Add netrc support to --bes_backend
69ee2d9 
Progress on bazelbuild#15856

Fixes bazelbuild#15928

Closes bazelbuild#15930.

PiperOrigin-RevId: 462379746
Change-Id: Ia7ae470bdcbd97c6cb42cc290c3891393ec9ce3a
@ckolli5 ckolli5 mentioned this pull request Jul 22, 2022
Closed
tjgq pushed a commit to tjgq/bazel that referenced this pull request Jul 25, 2022
@Yannic @tjgq
Add netrc support to --bes_backend
ee65573 
Progress on bazelbuild#15856

Fixes bazelbuild#15928

Closes bazelbuild#15930.

PiperOrigin-RevId: 462379746
Change-Id: Ia7ae470bdcbd97c6cb42cc290c3891393ec9ce3a
@tjgq tjgq mentioned this pull request Jul 25, 2022
Merged
ckolli5 pushed a commit that referenced this pull request Jul 25, 2022
@tjgq @Yannic
Add netrc support to --bes_backend (#15970)
96d23d3 
Progress on #15856

Fixes #15928

Closes #15930.

PiperOrigin-RevId: 462379746
Change-Id: Ia7ae470bdcbd97c6cb42cc290c3891393ec9ce3a

Co-authored-by: Yannic Bonenberger <yannic@engflow.com>
@ShreeM01 ShreeM01 mentioned this pull request Jul 25, 2022
Merged
ckolli5 added a commit that referenced this pull request Jul 25, 2022
@ShreeM01 @Yannic @ckolli5
Move newCredentialHelperProvider into GoogleAuthUtils (#15973)
508f185 
Follow-up on #15930 due to
#15906

Progress on #15856

Closes #15941.

PiperOrigin-RevId: 462580799
Change-Id: Ibac79ed68a0c87a4f34eb9d0729abb1552b44519

Co-authored-by: Yannic Bonenberger <yannic@engflow.com>
Co-authored-by: Chenchu K <ckolli@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tjgq tjgq tjgq approved these changes

Assignees
No one assigned
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

--bes_backend doesn't support reading credentials from .netrc
4 participants
@Yannic @sgowroji @tjgq @bazel-io