Hi all,
After playing a bit with Tristan's initial implementation [1] (see
[2]), I would like to make some changes to the proposal to allow more
flexibility for the plugins.
This was discussed previously in comments on [1], but I hope to
explain my ideas better in this proposal.
SourceMirrors are a new kind of plugins that provide a more general
means for projects to express their mirroring needs. Compared to the
previous proposal [3], I propose that buildstream don't impose any
special structure on the configuration data (except for the `name` and
`kind` keys). They are treated the same way as source plugins in this
regard, with a SourceMirror.COMMON_CONFIG_KEY member to be passed to
Node.validate_keys() alongside with plugin specific configuration.
Of course, `kind` would be optional to allow old mirror definitions to
keep working. Those would be handled by a new core SourceMirror plugin
that replicates the old behaviour.
This then leaves a hole in the proposal: how would the buildstream
core know which aliases the mirror can handle and which URLs the
mirror will use for each alias?
I would argue that the latter isn't important: buildstream doesn't
need to know the actual URL (especially that the plugin would then
transform anyway). For that, I would add a new method to SourceMirror
that returns the number of URLs that the mirror can provide for a
given alias. I don't have a proper name for it, so suggestions
welcome. I would find it clearer (and easier to name) if it just
returned a boolean, but the current mirrors can have more than one URL
per alias.
# Returns the number of URLs the mirror can provide for `alias`, zero if none.
def get_n_mirrors_for_alias(self, alias: str) -> int
Then, translate_url would get an index rather than the url.
# Return the `mirror_idx`th URL to use for downloading `source_url`.
# Any extra data that needs to be passed to Source implementations
# can be added to the `extra_data` dict.
def translate_url(
self,
alias: str,
source_url: str,
mirror_idx: int,
extra_data: Dict[str, Any],
) -> str
The extra_data parameter is taken from the "Authentication extension"
proposal [4] [5]. I think it would be better to have the SourceMirror
return the extra data, rather than have it as a parameter to be
modified. The above approach mirrors how extra_data would be handled
in Source.translate_url(). We can't do it as a return value in
Source.translate_url() because of backwards compatibility, but we
don't have the same restrictions on the new API.
Any thoughts?
There is another point that needs to be discussed, which is related to
applying mirrors to subprojects, but I'll leave that to a future post.
Cheers,
Abderrahim
[1] https://github.com/apache/buildstream/pull/1890
[2] https://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/merge_requests/18099
[3] https://lists.apache.org/thread/oxp2tmvd66wdwo3hzbgpc8x4hvrfyl5w
[4] https://lists.apache.org/thread/stdzzcgnl4oxwl9sgvg17zwhq4pb3l40
[5] https://github.com/apache/buildstream/pull/1895
