Re: [BuildStream] bst artifact show - PROPOSAL

[Tristan Van Berkom <tristan vanberkom codethink co uk>]

Hi James,

On Mon, 2019-08-05 at 08:38 +0100, James Ennis via buildstream-list wrote:
> Hello list,
>
> This post is in response to Ben's proposal of the `bst artifact show`
> command [0] and the replies from myself, Tristan and Sander [1].
>
> TL;DR
> 1. This command would be super useful and it will give us the ability to
>     be able to determine whether an artifact is cached remotely.
> 2. I mentioned in the thread that our current artifact subcommands
>     *only* deal with the local cache. I think it's *very* important that
>     we remain consistent, so, `bst artifact show` (with no options) will
>     show us what is cached locally. A `--remote` option or `all-remotes`
>     option will be required if we want to know whether an artifact is
>     cached remotely.
>         - Perhaps we could consider allowing for remotes to be declared
>           with an alias, therefore making the `--remote` option a bit
>           easier to use. Although, this should be an extension and not
>           block the work.
> 3. As Tristan suggested, the "local mode" of this function will probably
>     be what we had in mind for `bst artifact list`.
> 4. The command will support artifact refs, glob expressions and element
>     names.
> 5. We should be able to format the output.

[...]
> I've done my best to try and cover what I think all the use cases are
> and to address what was previously discussed in the older threads.
> However, there is a good chance I've overlooked some things so please
> respond with your opinions/improvements/objects etc :)

Thanks, You've done a great job summarizing :)

Just a few points:

  * You say that the command takes a list of artifact refs and element
    names as targets.

    I wonder if it should be allowed for these to be interchangeable,
    or if it will result in a difficult to load build graph with
    elements loaded from the active working tree interspersed with
    artifacts build from the same tree in a different state (same
    element more than once with different cache key in memory).

    We'll probably discover any limitations here in implementation.

  * There was no mention here of the `--deps` argument.

    I think it's important to have the `--deps` argument for this
    command, and just pointing out that this is going to require some
    work to load ArtifactElement objects recursively in memory.

  * Re-raising again that there was discussion around it being
    desirable to also address artifacts only by cache key (without
    needing to use the full ref, making the full ref name mostly
    just for user convenience).

    This is mostly because we don't want project and element names
    to be factored into the cache key (allowing greater re-usability
    of artifacts which are otherwise identical, and allowing renames
    of things without incurring cache misses).

    I think this is only tangentially related though (only because
    you said the command will take element names and full artifact
    refs) - I think it's the right call to support only full artifact
    refs and element names initially, and leave cache key only support
    out as a separate topic.

It's also not entirely clear to me how you intend to implement querying
of remotes, for instance, is there a shallow "pull queue" which ensures
we download just the shallow artifacts (metadata only) which runs first
before actually showing anything ?

Is the downloaded artifact metadata persisted in the local artifact
cache so that a subsequent show command need not re-download the
shallow artifacts ?

Cheers,
    -Tristan