Re: [BuildStream] Source cache plan

[Jürg Billeter <j bitron ch>]

Hi Raoul,

On Thu, 2019-01-17 at 12:56 +0000, Raoul Hidalgo Charman via BuildStream-list wrote:
> Source cache plan
> -----------------
>
> Source cache, originally raised in [4], will use both local and remote CAS's to
> store staged sources, preferentially trying to fetch from the remote cache(s),
> and if not present, fetch from the actual source.
>
> I suggest that the 'SourceCache' class be part of the context (similar to how
> 'ArtifactCache' is), and contains config related to source cache such as
> which remote(s) to use and the local CAS object. When the element class deals
> with sources it can now do it via the source cache rather than directly calling
> source methods.
>
> So far I think the source cache needs the following API which all take a source
> object as an argument:

I think we should aim for consistency with ArtifactCache/Element, at
least to some extent. The Element class is the one with the logic to
check/use ArtifactCache and ArtifactCache doesn't invoke any real
Element methods (besides getting cache key and name). Your proposal
does it the other way round, putting SourceCache in control.

Either approach could make sense, however, I think we should use the
same approach for both ArtifactCache and SourceCache, unless I'm
overlooking a fundamental difference.

Do you prefer the approach you've described? If so, do you think it
would make sense to also change ArtifactCache/Element?

I think an important discussion missing is how to extend the public
Source API to allow for efficient operation, without requiring plugins
to stage to a temporary directory. Although, maybe we can and should
tackle this as a follow-up step, possibly after switching to a CAS-
based workflow also for local builds (using BuildBox).

> * A sources '_preflight' isn't necessary if we only require the staged sources.
>    Should this check be removed in the cases where it's not needed?
> Some of these points may be optimisations that can be added later.

Can we be sure that `get_unique_key()` never requires `preflight()`?

Cheers,
Jürg