Hi, Le mer. 20 mars 2024 à 13:24, Jürg Billeter <[email protected]> a écrit : >[...] > > # Returns the number of URLs the mirror can provide for `alias`, zero if > > none. > > def get_n_mirrors_for_alias(self, alias: str) -> int > > If the support for multiple URLs is purely for compatibility with > existing non-plugin mirror definitions and we think it's not useful > with mirror plugins (i.e., an independent mirror plugin instance should > be configured instead), I'd rather keep this out of the public plugin > API.
I think it is. I always found it weird that a single mirror can provide more than one url for a given alias, when we can provide more than one mirror. But I'm open to more opinions. > One option would be to make the above method internal and make the > public method return a bool instead. Another option may be for the > public method to return a list of alias strings, not sure which variant > is a better fit. A third variant which I was considering earlier is to have a method of SourceMirror that plugins could call during their configure() to set the supported aliases. This way, plugins would still only implement translate_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 > > Here I'm leaning towards introducing some kind of translation object as > parameter to provide maximum flexibility. Besides making it possible to > provide access to the mirror index (or alias substitute URL) only via > an internal method or attribute, this would also make it possible to > support translation of unaliased URLs in the future, without having to > mark `alias` as an optional parameter. I'm not sure I see how it would be different in the case of no alias? The way I see it is that `TranslationObject.get_alias()` would have an `Optional[str]` as return type rather than `alias` being `Optional[str]`, which is essentially the same thing (if we change it during in a point release it would be a bit awkward as the plugin suddenly needs to handle it. I see it as valuable indeed if we ever wanted to give more information to plugins though. Cheers, Abderrahim
