Re: [BuildStream] CAS server resource names & instance names

[Jürg Billeter <j bitron ch>]

On Mon, 2018-12-10 at 11:58 +0000, Jim MacArthur via BuildStream-list wrote:
> On 10/12/2018 11:41, Jürg Billeter wrote:
> > I would not change anything, client or server side.
>
> We have to change something on the client side if we're to use CASCache 
> for remote execution, since some remote storage services (e.g. RBE) 
> require an instance name for storage.

Agreed. My statement was not quite clear, I meant that I would not
change anything for the instance_name="" case.

> I also don't see any mechanism for CASCache to know whether the server 
> supports instances other than by trying both and processing errors, so 
> I'd be strongly in favour of making the artifact cache cache accept 
> instance names in resource names, even if the instance name does not 
> become part of the key and they all map to the same namespace.

The client doesn't really need to know. It should just follow the
configuration. If it fails, an error should be reported. It should not
retry without instance name if the configuration includes an instance
name, in my opinion.

> The above paragraph Daniel has pointed out has some worrying assertions 
> in it:
>
> a) "the `instance_name` is an identifier, possibly containing multiple 
> path segments"
>
> b) "Anything after the `size` is ignored"
>
> Which I believe makes it impossible to exactly determine if a resource 
> name includes an instance name.

Maybe the sane thing would be for servers to either not use an instance
name (this would simply reject requests with instance name with
NOT_FOUND) or require an instance name (this would reject requests
without or with a wrong instance name with NOT_FOUND). Theoretically, a
server could support both as long as the instance name doesn't conflict
with the protocol-specified URL components, however, I don't think we
should or need to support this in bst-artifact-server, and the client
doesn't need to care about this.

In fact, I don't think we need to support multiple instance names at
all in bst-artifact-server. Let's leave this to BuildGrid. Extending it
to support a single non-empty instance name can be useful, at least for
testing, but that should also be reasonably straight forward. Or am I
missing something?

Cheers,
Jürg