Implement RemoteDownloader w/ `--experimental_remote_downloader` #10622

jmillikin-stripe · 2020-01-21T05:03:43Z

This is the Bazel client implementation of bazelbuild/proposals#160. It allows downloading of external dependencies to be delegated to a remote service.

TODOs:

Once Add Remote Asset API remote-apis#112 is merged, the vendored copy of bazelbuild/remote-apis should be updated. I've used a [WIP] placeholder for now.
If the general approach looks reasonable then I'll add tests. Currently I've been testing with an in-house implementation of the downloader server.

R: @buchgr @dslomov
CC: @EricBurnett @sstriker @ulfjack

buchgr · 2020-01-21T12:41:12Z

Thanks for the PR. I think the code should rather live in the downloader package next to the http implementation. I don't really see a good reason for it being in the remote package. WDYT?

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HashOutputStream.java

jmillikin-stripe · 2020-01-22T05:43:40Z

Thanks for the PR. I think the code should rather live in the downloader package next to the http implementation. I don't really see a good reason for it being in the remote package. WDYT?

When I tried to do this I hit circular dependency problems -- quite a lot of dependency problems, actually, because of how large the java_library targets are here.

I could try moving it back but it might require creating shim interfaces for most of the remote execution utilities (e.g. the tracing metadata injector).

I could also try creating a RemoteDownloader in downloader/, and then a separate GrpcRemoteDownloader extends RemoteDownloader that implements some part of the interface. This seemed unnecessary given that there's no plan for an HttpRemoteDownloader.

buchgr · 2020-01-24T13:53:07Z

It's hard for me to say how bad the circular dependency problem is. I'd much prefer if this functionality was implemented outside of the remote module. If that means splitting up large java_library targets into smaller ones then that's fine with me. I think we should treat the Fetcher service and remote execution/caching as separate. Both make sense to be used without the other.

For the flag naming how about --external_dep_proxy or any other ideas?

jmillikin-stripe · 2020-01-24T14:26:24Z

I think we should treat the Fetcher service and remote execution/caching as separate.

The remote downloader has a hard dependency on the remote CAS. It's not possible to use --remote_downloader without --remote_cache (and there's a check in RemoteOptions to enforce this).

For the flag naming how about --external_dep_proxy or any other ideas?

I think that would be confusing, given that this is not a proxy. It's a service that is told to download some external resource (from repository_ctx.download()) and put it in the CAS.

jmillikin-stripe · 2020-01-28T05:43:32Z

Done -- Rebased to master, un-WIP'd the remote-apis vendor commit, and relocated the GrpcRemoteDownloader to the downloader/ directory.

Does this look better? If so, I'll start on the tests.

jmillikin-stripe · 2020-01-31T09:56:29Z

Timed out waiting for response. I've added a unit test GrpcRemoteDownloaderTest to DownloaderTestSuite, cribbing heavily from HttpDownloaderTest and GrpcCacheClientTest.

Also removing [WIP] since the TODOs are resolved.

buchgr · 2020-01-31T09:58:55Z

...ain/java/com/google/devtools/build/lib/bazel/repository/downloader/GrpcRemoteDownloader.java

+    return out;
+  }
+
+  private static String authHeadersJson(Map<URI, Map<String, String>> authHeaders) {


How does this interact with the authentication methods from the remote module? Why do we want this?

ctx.download() allows arbitrary HTTP headers to be passed to the HTTP library for authenticating with the remote endpoint. This is part of the netrc support added in ... v2.0? v2.1? Fairly recently.

This code implements that by passing it along to the remote download server, which is free to use (or ignore) that data. The download server would use it when the place being downloaded from (e.g. example.com) needs special headers such as HTTP Basic Auth.

buchgr · 2020-01-31T10:00:18Z

src/main/java/com/google/devtools/build/lib/remote/BUILD

+        "ReferenceCountedChannel.java",
+        "RemoteRetrier.java",
+        "RemoteRetrierUtils.java",
+        "Retrier.java",


That's surprising. Why do you need this?

The RemoteRetrier class implements the logic of --remote_retries, which causes Bazel to retry failed gRPC calls sent to the remote executor/storage/downloader.

Let me be more clear: This target looks odd. Why do you need to exclude the sources? Is there a way to have more meaningful targets rather than one large one?

I would personally prefer smaller targets, and I also think it looks weird, but this is all pre-existing weirdness in Bazel.

//src/main/java/com/google/devtools/build/lib/remote is a glob of every Java file in the directory.

Since there is now a dependency of RemoteModule -> downloader -> RemoteRetrier, we need to have at least some of the Java files in remote/ be a separate target.

Other most-but-not-all-srcs style targets handle this case with globs that exclude some source files.

If you feel like adjusting the remote/ directory to have finer-grained targets that sounds like a good idea to me. I would prefer to avoid doing any major surgery to the Bazel build graph as part of this PR.

This is some unfortunate tech debt I'm willing to punt on temporarily. If you have any ideas on how to improve the situation/would like to clean up this package (ie, enumerating all files in the srcs list) it'd be much appreciated.

jmillikin-stripe · 2020-01-31T10:09:10Z

(rebased for #10679)

laurentlb · 2020-02-03T18:25:29Z

@buchgr, do you think this PR can be merged in time for Bazel 2.2?

(@jmillikin-stripe asked for it in #10133 (comment))

jmillikin-stripe · 2020-02-07T02:34:53Z

gentle ping @buchgr

Please let me know if this PR is too large to easily review, I can split some parts (like the remote-apis vendor update) into separate PRs if that would be better.

buchgr · 2020-02-07T10:11:59Z

Sorry I haven't forgotten about it. I am currently overloaded with an internal project. We'll ship it in 2.2.

jmillikin-stripe · 2020-02-19T03:10:47Z

I'm going to try splitting this up so it's easier to land incrementally, with the hopes of getting it into v2.3.

First subset is vendoring the updated remote-apis protos: #10818

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD

ulfjack · 2020-02-28T10:08:38Z

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloaderDelegate.java

+import java.util.Map;
+import java.util.Optional;
+
+public class DownloaderDelegate implements Downloader {


This class needs documentation

This is still open...

+1 - this makes it clear what's so special about this implementation. IIUC it's a wrapper around downloader that lets the implementation be swapped out?

...ain/java/com/google/devtools/build/lib/bazel/repository/downloader/GrpcRemoteDownloader.java

ulfjack · 2020-02-28T10:13:39Z

...ain/java/com/google/devtools/build/lib/bazel/repository/downloader/GrpcRemoteDownloader.java

+  }
+
+  @Override
+  public void download(


There's not enough documentation here. Minimum:

thread-safety (although that could also be covered in the interface)

deduplication

exceptions

ulfjack · 2020-02-28T10:15:55Z

...ain/java/com/google/devtools/build/lib/bazel/repository/downloader/GrpcRemoteDownloader.java

+        return null;
+      }));
+    } catch (StatusRuntimeException e) {
+      throw new IOException(e);


I really really dislike the decision by upstream grpc-java to use runtime exceptions. It makes it much harder to make the error handling correct.

However, we also need a clear policy for Bazel on how to handle these exceptions. In many cases, StatusRuntimeException is already wrapping an IOException, so this ends up with an IOException -> StatusRuntimeException -> IOException chain, which makes it hard to debug. My preference would be to rethrow the inner IOException if there is one, and only wrap if not.

I copied this exception handling from GrpcCacheClient, and I don't know enough about grpc-java to understand the exact issue you describe. Would it be possible for a Bazel dev to make the existing code act as you describe? Then I could copy it, and/or use a shared helper that does the right thing w.r.t. error handling.

ulfjack · 2020-02-28T11:30:52Z

src/main/java/com/google/devtools/build/lib/remote/RemoteDownloaderFactory.java

+
+public final class RemoteDownloaderFactory implements DownloaderFactory {
+
+  private volatile Downloader remoteDownloader;


Prefer synchronized over volatile. You almost never need it from a performance point of view, and it's much harder to reason about.

This logic is copied from the existing delegate/factory stacks, such as RepositoryRemoteExecutorFactoryDelegate. To be honest I have no idea why this level of indirection is necessary, and would be happy to have some guidance (or, like, direct commits to this PR) from someone who writes Java regularly.

I tried using synchronized instead, but the build fails:

src/main/java/com/google/devtools/build/lib/remote/RemoteDownloaderFactory.java:24: error: modifier synchronized not allowed here private synchronized Downloader remoteDownloader;

Oh, sorry. The synchronized goes on all methods that access the field, like so:

private Downloader remoteDownloader;

public synchronized void init(Downloader remoteDownloader) {
}

Yea, you can do something like...

@GuardedBy("this") private Downloader remoteDownloader;

Then add "synchronized" to all methods using it. This'll also let you get rid of the whole "remoteDownloader0" thing.

The volatile pattern in other files was likely an oversight.

src/main/java/com/google/devtools/build/lib/remote/RemoteDownloaderFactory.java

src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java

src/main/java/com/google/devtools/build/lib/remote/RemoteRetrierUtils.java

ulfjack · 2020-02-28T11:39:15Z

src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java

@@ -1589,7 +1597,8 @@ public BlazeRuntime build() throws AbruptExitException {
          productName,
          serverBuilder.getBuildEventArtifactUploaderMap(),
          serverBuilder.getAuthHeadersProvidersMap(),
-          serverBuilder.getRepositoryRemoteExecutorFactory());
+          serverBuilder.getRepositoryRemoteExecutorFactory(),
+          serverBuilder.getDownloaderFactory());


This looks wrong. This means it's not possible to have different Downloader factories for different builds in the same client. Shouldn't this be part of the CommandEnvironment instead?

I don't understand the question, but handling of the remote downloader should match that of the other remote capabilities (caching and execution). The calls to getRepositoryRemoteExecutorFactory() and getDownloaderFactory() belong together.

If they belong together somewhere else, could we do that separately? I'm very out of my depth when it comes to this level of the Bazel client architecture.

I think what ulf means is, assuming you're familiar with the client-server setup, the DownloaderFactory instance (and thus implementation) is tied to the lifetime of the server. It cannot be changed across different commands to the same server. I think this is fine since it's no worse than the current setup for other remote stuff, and there's only one impl.

jmillikin-stripe · 2020-03-02T02:26:45Z

@ulfjack Thank you so much for the review! I've addressed most comments, but there are a few places where I either didn't understand the request (I don't write much Java), or where the issue was not introduced by my change.

ulfjack

Note that I don't work at Google anymore, and I am spending most of my time starting my own company... providing a remote execution service and will also implement the remote asset API. However, it means that I can't currently promise any SLAs on reviews.

I think this is fine to merge. There are a few minor nits left, but they could also be addressed after the initial commit.

ulfjack · 2020-03-04T14:51:02Z

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloaderDelegate.java

+import java.util.Map;
+import java.util.Optional;
+
+public class DownloaderDelegate implements Downloader {


This is still open...

ulfjack · 2020-03-04T14:53:04Z

src/main/java/com/google/devtools/build/lib/remote/RemoteDownloaderFactory.java

+
+public final class RemoteDownloaderFactory implements DownloaderFactory {
+
+  private volatile Downloader remoteDownloader;


Oh, sorry. The synchronized goes on all methods that access the field, like so:

private Downloader remoteDownloader;

public synchronized void init(Downloader remoteDownloader) {
}

michajlo

I think conceptually everything seems fine - I trust that you understand the end-to-end goal here better than I and that you've been testing it locally. Most comments are around style and organization.

michajlo · 2020-03-04T19:38:01Z

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/Downloader.java

@@ -43,6 +43,7 @@ void download(
      List<URL> urls,
      Map<URI, Map<String, String>> authHeaders,
      Optional<Checksum> checksum,
+      String canonicalId,


can you document what this is?

The purpose and semantics of canonical_id is documented at https://docs.bazel.build/versions/master/skylark/lib/repository_ctx.html#download

Note that this field was originally present in the PR that added the Downloader interface. It got removed as part of the import into Google, and I just need back, because we have propagate this data to the backend somehow.

michajlo · 2020-03-04T19:40:07Z

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloaderDelegate.java

+import java.util.Map;
+import java.util.Optional;
+
+public class DownloaderDelegate implements Downloader {


+1 - this makes it clear what's so special about this implementation. IIUC it's a wrapper around downloader that lets the implementation be swapped out?

michajlo · 2020-03-04T19:43:49Z

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloaderDelegate.java

+      Map<String, String> clientEnv)
+      throws IOException, InterruptedException {
+    Downloader delegate = defaultDelegate;
+    Optional<DownloaderFactory> factory = delegateFactory;


i'm assuming this is in case delegateFactory changes mid-method? Is that actually expected? If so seems dubious... If not then just inline the call, which should simplify most of this method to...

Downloader delegate = delegateFactory.isPresent() ? delegateFactory.get().create().orElse(defaultDelegate) : defaultDelegate

michajlo · 2020-03-04T19:47:20Z

src/main/java/com/google/devtools/build/lib/remote/BUILD

+        "ReferenceCountedChannel.java",
+        "RemoteRetrier.java",
+        "RemoteRetrierUtils.java",
+        "Retrier.java",


This is some unfortunate tech debt I'm willing to punt on temporarily. If you have any ideas on how to improve the situation/would like to clean up this package (ie, enumerating all files in the srcs list) it'd be much appreciated.

michajlo · 2020-03-04T19:48:01Z

src/main/java/com/google/devtools/build/lib/remote/RemoteDownloaderFactory.java

+import com.google.devtools.build.lib.bazel.repository.downloader.DownloaderFactory;
+import java.util.Optional;
+
+public final class RemoteDownloaderFactory implements DownloaderFactory {


add some quick javadoc explaining what's so interesting about this impl.

michajlo · 2020-03-04T20:20:03Z

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HashOutputStream.java

+      actual = hasher.hash();
+    }
+    if (!code.equals(actual)) {
+      throw new UnrecoverableHttpException(


This side-effect is unfortunately hidden, but I guess there's prior art for this.

michajlo · 2020-03-04T20:26:47Z

src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java

+
+  @BeforeClass
+  public static void beforeEverything() {
+    retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));


why not put this in the before and after? Otherwise we can leak threads across tests within this test class

I copy-pasted this from the existing downloader test case, and don't have enough context in Bazel to know whether there's a pre-existing bug here.

michajlo · 2020-03-04T20:27:56Z

src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java

+  private static byte[] downloadBlob(
+      GrpcRemoteDownloader downloader, URL url, Optional<Checksum> checksum)
+      throws IOException, InterruptedException {
+    final List<URL> urls = ImmutableList.<URL>of(url);


Modern java should be smart enough that you can leave the "" out of "ImmutableList.of(...)", so rm it. Same for other occurrences where the compiler is fine with it.

Done in forked PR #10914

michajlo · 2020-03-04T20:30:12Z

src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java

+  }
+
+  private GrpcRemoteDownloader newDownloader(RemoteCacheClient cacheClient) throws IOException {
+    final RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class);


btw, final shouldn't be necessary on most local variables, but it's not necessary to remove

michajlo · 2020-03-04T20:32:24Z

src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java

+
+    getFromFuture(cacheClient.uploadBlob(contentDigest, ByteString.copyFromUtf8("wrong content")));
+
+    IOException e = assertThrows(UnrecoverableHttpException.class, () -> downloadBlob(


Fix formatting to improve readability:

IOException e = assertThrows( UnrecoverableHttpException.class, () -> downloadBlob( downloader, ..., ...));

Done in forked PR #10914

jmillikin-stripe · 2020-03-06T08:10:13Z

Per Dmitry's latest comment on #10742, it sounds like this has missed the merge window for v3.0. I am going to take the parts that the reviewer(s) seem happy with and them into a separate PR (#10914), in the hopes of trimming this one down further.

katre · 2020-03-06T13:55:29Z

Left comments on #10914. What's remaining in this PR after that? Can you give a high-level description?

jmillikin-stripe · 2020-03-07T01:00:35Z

After that PR is merged, this one would be just the plumbing of the new downloader into RemoteModule, BazelRuntime, and so on. I'm hopeful that since I'm just copying what Bazel is already doing, I can link each new code block to the existing code and thus simplify the reviewer's task.

@michajlo

Extracted from #10622 Per discussion on that PR, there's still some unanswered questions about how exactly we plumb the new `Downloader` type into `RemoteModule`. And per #10742 (comment), it is unlikely that even heroic effort from me will get the full end-to-end functionality into v3.0. Given this, to simplify the review, I'm taking some of the bits the reviewer is happy with and moving them to a separate PR. After merger, `GrpcRemoteDownloader` and its tests will exist in the source tree, but will not yet be available as CLI options. R: @michajlo CC: @adunham-stripe @dslomov @EricBurnett @philwo @sstriker Closes #10914. PiperOrigin-RevId: 299908615

katre · 2020-03-09T19:59:05Z

#10914 is now merged, do you want to rebase this and I will review the remainder?

jmillikin-stripe · 2020-03-10T04:04:41Z

Rebased to master.

Per earlier comments, I've rewritten this final commit. Instead of using the existing design of RemoteRepositoryRemoteExecutorFactory as a reference, it passes around a Supplier<Downloader>.

I don't know if the new version is as idiomatic for Java, but I find it easier to follow. Please let me know if you'd prefer the previous way.

katre

This looks fine overall, just one question.

katre · 2020-03-10T13:15:11Z

src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java

+            repoContext,
+            cacheClient,
+            remoteOptions));
+        downloaderChannel.release();


Why the immediate release after the retain?

Trying to match the existing code. Ctrl+F new ByteStreamUploader( or new GrpcRemoteExecutor( for where I'm looking as examples.

Thanks for the explanation. I'll import this today and try to cut a new RC of 3.0.0.

@michajlo

Extracted from #10622 Per discussion on that PR, there's still some unanswered questions about how exactly we plumb the new `Downloader` type into `RemoteModule`. And per #10742 (comment), it is unlikely that even heroic effort from me will get the full end-to-end functionality into v3.0. Given this, to simplify the review, I'm taking some of the bits the reviewer is happy with and moving them to a separate PR. After merger, `GrpcRemoteDownloader` and its tests will exist in the source tree, but will not yet be available as CLI options. R: @michajlo CC: @adunham-stripe @dslomov @EricBurnett @philwo @sstriker Closes #10914. PiperOrigin-RevId: 299908615

@buchgr

This is the Bazel client implementation of bazelbuild/proposals#160. It allows downloading of external dependencies to be delegated to a remote service. TODOs: - [x] Once bazelbuild/remote-apis#112 is merged, the vendored copy of `bazelbuild/remote-apis` should be updated. I've used a [WIP] placeholder for now. - [x] If the general approach looks reasonable then I'll add tests. Currently I've been testing with an in-house implementation of the downloader server. R: @buchgr @dslomov CC: @EricBurnett @sstriker @ulfjack Closes #10622. PiperOrigin-RevId: 300116716

jmillikin-stripe · 2020-03-10T22:08:22Z

@katre Thank you so much for your help! Both for the review, and for handling the cherry-pick.

I no longer have Peer Bonus access, but if you're ever in Tokyo I'll buy you lunch or something :P

Qinusty · 2020-07-29T12:44:08Z

src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java

@@ -160,6 +172,13 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
    boolean enableHttpCache = RemoteCacheClientFactory.isHttpCache(remoteOptions);
    boolean enableGrpcCache = GrpcCacheClient.isRemoteCacheOptions(remoteOptions);
    boolean enableRemoteExecution = shouldEnableRemoteExecution(remoteOptions);
+    boolean enableRemoteDownloader = shouldEnableRemoteDownloader(remoteOptions);
+
+    if (enableRemoteDownloader && !enableGrpcCache) {


This seems to require --remote_cache be set alongside --remote_executor are there any side effects which could arise from changing this to the below snippet?

if (enableRemoteDownloader && (!enableGrpcCache || !enableRemoteExecution)) {

Remote execution is not required to use remote downloads. Remote caching (specifically gRPC remote caching) is required, because that's where the downloads get placed.

However --remote_cache is not usually required for the use of --remote_executor. I assume bazel has some defaulting behaviour for the cache endpoint to be the executor endpoint. This enforces a flag requirement for the combination of these features.

Remote execution with the use of the remote downloader requires all 3 flags.

I've opened #11913 to track this issue.

purkhusid · 2022-02-26T19:26:59Z

@jmillikin Is there any way to invalidate the Remote Downloader cache? I've tried using --remote_instance_name but it does not seem to affect the Remote Downloader cache.

jmillikin · 2022-02-26T22:02:08Z

I think that would be a question for the author of your remote downloader implementation. The Bazel client only uses one RPC, which is approximately "download file and return the CAS checksum + size". Other RPCs for cache management and invalidation are implementation-specific.

purkhusid · 2022-02-28T09:37:59Z

@jmillikin Ok, but I wonder if it might be worth a feature request to make the Remote Assets API sensitive to the --remote_instance_name flag? That could be implemented as a client side feature, right?

jmillikin-stripe requested review from buchgr and ola-rozenfeld as code owners January 21, 2020 05:03

googlebot added the cla: yes label Jan 21, 2020

This was referenced Jan 21, 2020

Unify remote downloader proposals (remote repo cache, remote fetch api) bazelbuild/proposals#160

Merged

Allow using remote cache for repository cache #6359

Open

buchgr reviewed Jan 21, 2020

View reviewed changes

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HashOutputStream.java Outdated Show resolved Hide resolved

jin assigned buchgr Jan 23, 2020

jin added the team-Remote-Exec Issues and PRs for the Execution (Remote) team label Jan 23, 2020

jmillikin-stripe force-pushed the remote-downloader-impl branch 3 times, most recently from 8a3f5a8 to 33bb551 Compare January 28, 2020 05:42

jmillikin-stripe changed the title ~~[WIP] Implement RemoteDownloader w/ --experimental_remote_downloader~~ Implement RemoteDownloader w/ --experimental_remote_downloader Jan 31, 2020

buchgr reviewed Jan 31, 2020

View reviewed changes

jmillikin-stripe force-pushed the remote-downloader-impl branch from b9d6fd7 to d81b062 Compare January 31, 2020 10:09

jmillikin-stripe mentioned this pull request Feb 3, 2020

Release 2.2 (stable) - February 2020 #10133

Closed

10 tasks

This was referenced Feb 19, 2020

use ASCII apostrophes in remote_asset.proto bazelbuild/remote-apis#115

Merged

Update vendored @remoteapis for PR bazelbuild/remote-apis#112 #10818

Closed

robbertvanginkel mentioned this pull request Feb 20, 2020

Release 3.0 - March 2020 #10742

Closed

ulfjack requested changes Feb 28, 2020

View reviewed changes

jmillikin-stripe force-pushed the remote-downloader-impl branch from 0878448 to 0caf2c4 Compare March 2, 2020 02:22

katre requested review from michajlo and removed request for ola-rozenfeld March 4, 2020 14:22

ulfjack reviewed Mar 4, 2020

View reviewed changes

michajlo reviewed Mar 4, 2020

View reviewed changes

jmillikin-stripe force-pushed the remote-downloader-impl branch 2 times, most recently from cf9a90e to b3d1ddb Compare March 6, 2020 08:03

jmillikin-stripe mentioned this pull request Mar 6, 2020

Implementation (but not plumbing) of the gRPC remote downloader #10914

Closed

Finish plumbing GrpcRemoteDownloader into RemoteModule.

5f8553e

jmillikin-stripe force-pushed the remote-downloader-impl branch from b3d1ddb to 5f8553e Compare March 10, 2020 03:35

katre reviewed Mar 10, 2020

View reviewed changes

bazel-io closed this in 05de852 Mar 10, 2020

jmillikin-stripe deleted the remote-downloader-impl branch March 10, 2020 22:07

Qinusty reviewed Jul 29, 2020

View reviewed changes

aherrmann mentioned this pull request Jan 8, 2021

Move macOS CI to GitHub Actions tweag/rules_haskell#1460

Merged

rockwotj mentioned this pull request May 14, 2021

Feature request: external dependency caching using remote gRPC protocol #2557

Closed


		public final class RemoteDownloaderFactory implements DownloaderFactory {

		private volatile Downloader remoteDownloader;


		getFromFuture(cacheClient.uploadBlob(contentDigest, ByteString.copyFromUtf8("wrong content")));

		IOException e = assertThrows(UnrecoverableHttpException.class, () -> downloadBlob(

Implement RemoteDownloader w/ --experimental_remote_downloader #10622

Implement RemoteDownloader w/ --experimental_remote_downloader #10622

Conversation

jmillikin-stripe commented Jan 21, 2020 • edited Loading

buchgr commented Jan 21, 2020

jmillikin-stripe commented Jan 22, 2020

buchgr commented Jan 24, 2020 • edited Loading

jmillikin-stripe commented Jan 24, 2020 • edited Loading

jmillikin-stripe commented Jan 28, 2020

jmillikin-stripe commented Jan 31, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmillikin-stripe Jan 31, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmillikin-stripe commented Jan 31, 2020

laurentlb commented Feb 3, 2020

jmillikin-stripe commented Feb 7, 2020

buchgr commented Feb 7, 2020

jmillikin-stripe commented Feb 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmillikin-stripe commented Mar 2, 2020

ulfjack left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michajlo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmillikin-stripe commented Mar 6, 2020

katre commented Mar 6, 2020

jmillikin-stripe commented Mar 7, 2020

katre commented Mar 9, 2020

jmillikin-stripe commented Mar 10, 2020

katre left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmillikin-stripe commented Mar 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

purkhusid commented Feb 26, 2022

jmillikin commented Feb 26, 2022

purkhusid commented Feb 28, 2022

Implement RemoteDownloader w/ `--experimental_remote_downloader` #10622

Implement RemoteDownloader w/ `--experimental_remote_downloader` #10622

jmillikin-stripe commented Jan 21, 2020 •

edited

Loading

buchgr commented Jan 24, 2020 •

edited

Loading

jmillikin-stripe commented Jan 24, 2020 •

edited

Loading

jmillikin-stripe Jan 31, 2020 •

edited

Loading