Plugin support to map image to downloadable url #1090
Conversation
Should be opened to other archictectures and formats. Modified testcloud to use it for x86_64 and box|qcow2 which are currently supported (non-intel arch support is work in progress)
|
I went with flow: Get all mapping plugins, call first one's convert for value. If it returns None, try next plugin. If returns string, use it as URL. Each plugin should make sure to return None as soon as possible (e.g. wrong arch, wrong type, wrong name). What I'm not sure about guidance what to do when plugin should be able to map image value but can't. (e.g. image is nightly compose which has been removed). Should plugin raise GeneralError or return None and let some other plugin map it? Signature |
There was a problem hiding this comment.
In general looks good to me. A couple of minor comments added. I'm also thinking about our recent discussion to provide a way for "consistent" image names. Is that something that should be handled around here? Or is this a different topic? Quick braindump of possible usage:
image = tmt.Image("rhel8")
image = tmt.Image("rhel-8")
image = tmt.Image("RHEL8")Also what are the expectation around the other proposed formats?
rhel-8.nightly (bleeding edge)
rhel-8.tagged (tested by rtt)
rhel-8.tested (how exactly tested?)
rhel-8.latest (what exactly is latest? released? tagged? nightly?)
Plus specifying minor version:
rhel-8.6
rhel-8.6.z
Should everything from above be handled by Image2URLConvertor classes?
Also what about the possibily to choose the output format?
image = tmt.Image("centos-8")
image.url() # return url for image download
image.container_image() # appropriate podman image name (centos:stream8)Probably will need some more discussion & brainstorming?
| name, arch='x86_64', format=[ | ||
| 'box', 'qcow2']) | ||
| if url is not None: | ||
| return url |
There was a problem hiding this comment.
Condition not needed. The url would be returned even if None because of line 381.
| class Image2URLConvertor(object): | ||
| """ Common parent for image to url conversion plugins """ | ||
| @staticmethod | ||
| def convert(value, arch=None, format=[]): |
There was a problem hiding this comment.
| def convert(value, arch=None, format=[]): | |
| def convert(value, arch=None, format=None): |
Using empty list as the default value is not recommended as it can cause problem with repeated calls. See also d4f41a5.
| raise NotImplementedError() | ||
|
|
||
|
|
||
| def get_image_url(value, arch, format): |
There was a problem hiding this comment.
The value sounds very generic. Is it rather an image name?
|
I'll update PR with Later we can add 'generic name' support which will do translation |
Should be opened to other archictectures and formats.
Modified testcloud to use it for x86_64 and box|qcow2 which are
currently supported (non-intel arch support is work in progress)