Launchpad itself

Merge ~ilasc/launchpad:distribution-mirror-apis into launchpad:master

Proposed by Ioana Lasc on 2022-04-01

Status:	Merged
Approved by:	Ioana Lasc on 2022-04-08
Approved revision:	c0e13f5e4571c472438ede0e107551a2217d2659
Merge reported by:	Otto Co-Pilot
Merged at revision:	not available
Proposed branch:	~ilasc/launchpad:distribution-mirror-apis
Merge into:	launchpad:master
Diff against target:	572 lines (+171/-115) 11 files modified lib/lp/registry/browser/distribution.py (+1/-2) lib/lp/registry/doc/distribution-mirror.txt (+6/-5) lib/lp/registry/interfaces/distribution.py (+14/-0) lib/lp/registry/interfaces/distributionmirror.py (+3/-9) lib/lp/registry/model/distribution.py (+47/-0) lib/lp/registry/model/distributionmirror.py (+0/-50) lib/lp/registry/stories/webservice/xx-distribution-mirror.txt (+6/-0) lib/lp/registry/stories/webservice/xx-distribution.txt (+2/-0) lib/lp/registry/tests/test_distribution.py (+43/-0) lib/lp/registry/tests/test_distributionmirror.py (+45/-43) scripts/cache-country-mirrors.py (+4/-6)
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Colin Watson (community)		2022-04-01	Approve on 2022-04-08
Review via email: mp+418173@code.qastaging.launchpad.net

Commit message

Expose base_url and getBestMirrorsForCountry over API

Revision history for this message

Colin Watson (cjwatson) wrote on 2022-04-01:

This looks fine as far as it goes, but I think we ought to consider the interface a bit before exporting it on the webservice API and thus somewhat setting it in stone. (The basic problem here predates you, but this would be a good time to fix it.)

`DistributionMirrorSet.getBestMirrorsForCountry` only takes the country and the mirror type. As a result, `cache-country-mirrors.py` and hence mirrors.ubuntu.com can potentially end up listing non-Ubuntu mirrors. There are four non-Ubuntu mirrors in the production database at the moment. As it happens, they all have `content=RELEASE` rather than `content=ARCHIVE` so `cache-country-mirrors.py` doesn't consider them, but there's no intrinsic reason this has to be the case; this would be a good time to fix our code to be properly distribution-scoped before it becomes a problem.

`getBestMirrorsForCountry` isn't called from very many places right now, so this should be easy enough to fix. I suggest moving this method to `Distribution` (since it already has various exported methods related to mirrors), which would allow adding a `DistributionMirror.distribution == self` condition to its `base_query`. `DistributionCountryArchiveMirrorsView` can just call it on `self.context`, while `CacheCountryMirrors` can use `getUtility(ILaunchpadCelebrities).ubuntu` for the time being. Once you've done that, you can revert the bits of this MP that export `IDistributionMirrorSet` as a webservice collection, and instead just export this one method on `IDistribution`, which will be cleaner and more future-proof.