Differential D1055

Improvements to the dist-git directive.
ClosedPublic
Actions

Authored by • ralph2 on Nov 16 2016, 7:13 PM.

Details

Reviewers
tflink
Commits
rLTRN936083475171: Improvements to the dist-git directive.
Summary

Our use case is that we want to be able to run the modlint tool in
response to dist-git commits to modulemd.yaml files. modlint is kind
of like rpmlint, except for module definitions.

We want to run those checks in response to dist-git commits via
taskotron-trigger.

We were excited to see that a dist-git directive already exists, but it
assumed that you were specifying your request in terms of an rpm NEVR
which we do not have in our use case.

This changes opens it up to be able to pull down any file from any
namespace of dist-git repo, on any branch. It is up to the caller of
the dist-git directive to specify all of that information.

Test Plan
Run the test suite.

Diff Detail

Repository
rLTRN libtaskotron
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
• ralph2 retitled this revision from to Improvements to the dist-git directive..Nov 16 2016, 7:13 PM
• ralph2 updated this object.
• ralph2 edited the test plan for this revision. (Show Details)
• ralph2 added a reviewer: tflink.
Closed by commit rLTRN936083475171: Improvements to the dist-git directive. (authored by Ralph Bean <rbean@redhat.com>). · Explain WhyDec 12 2016, 2:54 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 2722

View Options

libtaskotron/directives/distgit_directive.py

Show All 10 Lines
11description: | 11​description: |
12 Download files from Fedora package repository (usually called 'distgit') hosted at 12​ Download files from Fedora package repository (usually called 'distgit') hosted at
13 http://pkgs.fedoraproject.org/. Any files hosted at that repository can be downloaded for a 13​ http://pkgs.fedoraproject.org/. Any files hosted at that repository can be downloaded for a
14 chosen package. 14​ chosen package.
15parameters: 15​parameters:
16 package: 16​ package:
17 required: true 17​ required: true
18 description: | 18​ description: |
19 N(E)VR of a package. Package dist tag determines the git branch used for file download. 19​ Name of a package.
20 Example: ``xchat-2.8.8-21.fc20`` 20​ Example: ``xchat``
21 type: str 21​ type: str
22​ gitref:
23​ required: false
24​ description: |
25​ A git ref to check out. May be a branch, tag, or commit hash.
26​ Example: ``f24`` or ``04164165a840405e6bb5acc54a51e22346d84e0d``
27​ type: str
28​ default: master
29​ namespace:
30​ required: false
31​ description: |
32​ dist-git namespace to use when constructing the url to clone.
33​ Example: ``modules``, ``rpms`` or ``docker``
34​ type: str
35​ default: rpms
22 path: 36​ path:
23 required: true 37​ required: true
24 description: | 38​ description: |
25 files (directories not supported at the moment) to be downloaded from distgit. 39​ files (directories not supported at the moment) to be downloaded from distgit.
26 Example: ``[xchat.spec]`` 40​ Example: ``[xchat.spec]``
27 type: list of str 41​ type: list of str
28 localpath: 42​ localpath:
29 required: false 43​ required: false
30 description: | 44​ description: |
31 a local path of downloaded file. If not provided, path from distgit will be used. 45​ a local path of downloaded file. If not provided, path from distgit will be used.
32 Example: ``[specs/xchat.spec]`` 46​ Example: ``[specs/xchat.spec]``
33 type: list of str 47​ type: list of str
34 target_dir: 48​ target_dir:
35 required: false 49​ required: false
36 description: directory into which to download files 50​ description: directory into which to download files
37 type: str 51​ type: str
38 default: ${workdir} 52​ default: ${workdir}
53​ baseurl:
54​ required: false
55​ description: |
56​ The baseurl to use for dist-git. Defaults to Fedora's production instance.
57​ Example: ``http://pkgs.stg.fedoraproject.org``
58​ type: str
59​ default: http://pkgs.fedoraproject.org
39returns: | 60​returns: |
40 A dictionary containing following items: 61​ A dictionary containing following items:
41 62​
42 * `downloaded_files`: (list of str) a list of local filenames of the downloaded 63​ * `downloaded_files`: (list of str) a list of local filenames of the downloaded
43 files 64​ files
44raises: | 65​raises: |
45 * :class:`.TaskotronRemoteError`: if downloading failed 66​ * :class:`.TaskotronRemoteError`: if downloading failed
46 * :class:`.TaskotronValueError`: if path and localpath are not lists or are not of the same 67​ * :class:`.TaskotronValueError`: if path and localpath are not lists or are not of the same
47 length 68​ length
48 * :class:`.TaskotronDirectiveError`: if package or path is missing 69​ * :class:`.TaskotronDirectiveError`: if package or path is missing
49version_added: 0.3.16 70​version_added: 0.3.16
50""" 71​"""
51 72​
52EXAMPLES = """ 73​EXAMPLES = """
53A task needs to download a spec file and httpd configuration check to run a check 74​A task needs to download a spec file and httpd configuration check to run a check
54on those files:: 75​on those files::
55 76​
56 - name: download spec file and httpd conf 77​ - name: download spec file and httpd conf
57 distgit: 78​ distgit:
58 package: yourls-1.7-3.20150410gitabc7d6c.fc22 79​ package: yourls
80​ gitref: f22
59 path: 81​ path:
60 - yourls.spec 82​ - yourls.spec
61 - yourls-httpd.conf 83​ - yourls-httpd.conf
62 localpath: 84​ localpath:
63 - download/yourls_downloaded.spec 85​ - download/yourls_downloaded.spec
64 - download/yourls-httpd_downloaded.conf 86​ - download/yourls-httpd_downloaded.conf
65""" 87​"""
66 88​
67import os.path 89​import os.path
68 90​
69from libtaskotron.directives import BaseDirective 91​from libtaskotron.directives import BaseDirective
70from libtaskotron import file_utils, python_utils 92​from libtaskotron import file_utils, python_utils
71from libtaskotron.ext.fedora import rpm_utils ​
72 93​
73import libtaskotron.exceptions as exc 94​import libtaskotron.exceptions as exc
74 95​
75 96​
76 97​
77directive_class = 'DistGitDirective' 98​directive_class = 'DistGitDirective'
78 99​
79URL_FMT = 'http://pkgs.fedoraproject.org/cgit/%s.git/plain/%s?h=%s' 100​URL_FMT = '{baseurl}/cgit/{namespace}/{package}.git/plain/{path}?h={gitref}'
80 101​
81 102​
82class DistGitDirective(BaseDirective): 103​class DistGitDirective(BaseDirective):
83 def __init__(self): 104​ def __init__(self):
84 super(DistGitDirective, self).__init__() 105​ super(DistGitDirective, self).__init__()
85 106​
86 def _download_file(self, package, path, localpath): ​
87 pkgname = rpm_utils.rpmformat(package, fmt='n') ​
88 dist_tag = rpm_utils.get_dist_tag(package) ​
89 # fc22 -> f22 conversion, since that's how branches are called ​
90 branch = dist_tag.replace('c', '') ​
91 url = URL_FMT % (pkgname, path, branch) ​
92 ​
93 return file_utils.download(url, '.', localpath) ​
94 ​
95 def process(self, params, arg_data): 107​ def process(self, params, arg_data):
96 if 'package' not in params or 'path' not in params: 108​ if 'package' not in params or 'path' not in params:
97 detected_args = ', '.join(params.keys()) 109​ detected_args = ', '.join(params.keys())
98 raise exc.TaskotronDirectiveError( 110​ raise exc.TaskotronDirectiveError(
99 "The distgit directive requires 'package' and 'path' arguments." 111​ "The distgit directive requires 'package' and 'path' arguments."
100 "Detected arguments: %s" % detected_args) 112​ "Detected arguments: %s" % detected_args)
101 113​
102 if 'target_dir' not in params: 114​ # Assign defaults
103 target_dir = arg_data['workdir'] 115​ gitref = params.get('gitref', 'master')
104 else: 116​ namespace = params.get('namespace', 'rpms')
105 target_dir = params['target_dir'] 117​ baseurl = params.get('baseurl', 'http://pkgs.fedoraproject.org')
118​ target_dir = params.get('target_dir', arg_data['workdir'])
119​
106 120​
107 if not python_utils.iterable(params['path']): 121​ if not python_utils.iterable(params['path']):
108 raise exc.TaskotronValueError("Incorrect value type of the 'path' argument: " 122​ raise exc.TaskotronValueError("Incorrect value type of the 'path' argument: "
109 "%s" % type(params['path'])) 123​ "%s" % type(params['path']))
110 124​
111 target_path = params['path'] 125​ target_path = params['path']
112 output_data = {} 126​ output_data = {}
113 127​
114 if 'localpath' in params: 128​ if 'localpath' in params:
115 if not python_utils.iterable(params['localpath']): 129​ if not python_utils.iterable(params['localpath']):
116 raise exc.TaskotronValueError("Incorrect value type of the 'localpath' argument: " 130​ raise exc.TaskotronValueError("Incorrect value type of the 'localpath' argument: "
117 "%s" % type(params['path'])) 131​ "%s" % type(params['path']))
118 132​
119 if not len(params['path']) == len(params['localpath']): 133​ if not len(params['path']) == len(params['localpath']):
120 raise exc.TaskotronValueError('path and localpath lists must be of the same ' 134​ raise exc.TaskotronValueError('path and localpath lists must be of the same '
121 'length.') 135​ 'length.')
122 136​
123 target_path = params['localpath'] 137​ target_path = params['localpath']
124 138​
125 139​ format_fields = {
140​ 'package': params['package'],
141​ 'gitref': gitref,
142​ 'namespace': namespace,
143​ 'baseurl': baseurl,
144​ }
126 output_data['downloaded_files'] = [] 145​ output_data['downloaded_files'] = []
127 for path, localpath in zip(params['path'], target_path): 146​ for path, localpath in zip(params['path'], target_path):
128 localpath = os.path.join(target_dir, localpath) 147​ localpath = os.path.join(target_dir, localpath)
129 file_utils.makedirs(os.path.dirname(localpath)) 148​ file_utils.makedirs(os.path.dirname(localpath))
149​ url = URL_FMT.format(path=path, **format_fields)
130 output_data['downloaded_files'].append( 150​ output_data['downloaded_files'].append(
131 self._download_file(params['package'], path, localpath) 151​ file_utils.download(url, '.', localpath)
132 ) 152​ )
133 153​
134 return output_data 154​ return output_data
View Options

testing/test_distgit_directive.py

1# -*- coding: utf-8 -*- 1​# -*- coding: utf-8 -*-
2# Copyright 2009-2015, Red Hat, Inc. 2​# Copyright 2009-2015, Red Hat, Inc.
3# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+> 3​# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+>
4# See the LICENSE file for more details on Licensing 4​# See the LICENSE file for more details on Licensing
5 5​
6import os.path 6​import os.path
7import pytest 7​import pytest
8from dingus import Dingus 8​from dingus import Dingus
9 9​
10from libtaskotron import file_utils 10​from libtaskotron import file_utils
11from libtaskotron.directives import distgit_directive 11​from libtaskotron.directives import distgit_directive
12import libtaskotron.exceptions as exc 12​import libtaskotron.exceptions as exc
13 13​
14 14​
15class TestDistGitDirective(): 15​class TestDistGitDirective():
16 def setup_method(self, method): 16​ def setup_method(self, method):
17 self.ref_nvr = 'foo-1.2-3.fc99' 17​ self.ref_name = 'foo'
18 self.ref_path = ['foo.spec'] 18​ self.ref_path = ['foo.spec']
19 self.ref_localpath = ['local.foo.spec'] 19​ self.ref_localpath = ['local.foo.spec']
20 self.ref_branch = 'f99' 20​ self.ref_branch = 'f99'
21 self.ref_arg_data = {'workdir': '/var/tmp/foo'} 21​ self.ref_arg_data = {'workdir': '/var/tmp/foo'}
22 self.helper = distgit_directive.DistGitDirective() 22​ self.helper = distgit_directive.DistGitDirective()
23 23​
24 def _get_url(self, path): 24​ def _get_url(self, path):
25 return distgit_directive.URL_FMT % ('foo', path, self.ref_branch) 25​ return distgit_directive.URL_FMT.format(
26​ baseurl='http://pkgs.fedoraproject.org',
27​ namespace='rpms',
28​ package='foo',
29​ path=path,
30​ gitref=self.ref_branch)
26 31​
27 def test_parse_download_command(self, monkeypatch): 32​ def test_parse_download_command(self, monkeypatch):
28 ref_input = {'action': 'download', 33​ ref_input = {'action': 'download',
29 'package': self.ref_nvr, 34​ 'package': self.ref_name,
35​ 'gitref': self.ref_branch,
30 'path': self.ref_path} 36​ 'path': self.ref_path}
31 37​
32 monkeypatch.setattr(file_utils, 'download', Dingus()) 38​ monkeypatch.setattr(file_utils, 'download', Dingus())
33 39​
34 self.helper.process(ref_input, self.ref_arg_data) 40​ self.helper.process(ref_input, self.ref_arg_data)
35 41​
36 download_calls = file_utils.download.calls 42​ download_calls = file_utils.download.calls
37 43​
38 assert len(download_calls) == 1 44​ assert len(download_calls) == 1
39 assert download_calls[0][1][0] == self._get_url(self.ref_path[0]) 45​ assert download_calls[0][1][0] == self._get_url(self.ref_path[0])
40 assert download_calls[0][1][2] == os.path.join(self.ref_arg_data['workdir'], 46​ assert download_calls[0][1][2] == os.path.join(self.ref_arg_data['workdir'],
41 self.ref_path[0]) 47​ self.ref_path[0])
42 48​
43 def test_parse_download_command_localpath(self, monkeypatch): 49​ def test_parse_download_command_localpath(self, monkeypatch):
44 ref_input = {'action': 'download', 50​ ref_input = {'action': 'download',
45 'package': self.ref_nvr, 51​ 'package': self.ref_name,
52​ 'gitref': self.ref_branch,
46 'path': self.ref_path, 53​ 'path': self.ref_path,
47 'localpath': self.ref_localpath} 54​ 'localpath': self.ref_localpath}
48 55​
49 monkeypatch.setattr(file_utils, 'download', Dingus()) 56​ monkeypatch.setattr(file_utils, 'download', Dingus())
50 57​
51 self.helper.process(ref_input, self.ref_arg_data) 58​ self.helper.process(ref_input, self.ref_arg_data)
52 59​
53 download_calls = file_utils.download.calls 60​ download_calls = file_utils.download.calls
54 61​
55 assert len(download_calls) == 1 62​ assert len(download_calls) == 1
56 assert download_calls[0][1][0] == self._get_url(self.ref_path[0]) 63​ assert download_calls[0][1][0] == self._get_url(self.ref_path[0])
57 assert download_calls[0][1][2] == os.path.join(self.ref_arg_data['workdir'], 64​ assert download_calls[0][1][2] == os.path.join(self.ref_arg_data['workdir'],
58 self.ref_localpath[0]) 65​ self.ref_localpath[0])
59 66​
60 def test_parse_download_command_multiple_localpath(self, monkeypatch): 67​ def test_parse_download_command_multiple_localpath(self, monkeypatch):
61 ref_path = ['file1', 'file2'] 68​ ref_path = ['file1', 'file2']
62 ref_localpath = ['file3', 'file4'] 69​ ref_localpath = ['file3', 'file4']
63 ref_input = {'action': 'download', 70​ ref_input = {'action': 'download',
64 'package': self.ref_nvr, 71​ 'package': self.ref_name,
72​ 'gitref': self.ref_branch,
65 'path': ref_path, 73​ 'path': ref_path,
66 'localpath': ref_localpath} 74​ 'localpath': ref_localpath}
67 75​
68 monkeypatch.setattr(file_utils, 'download', Dingus()) 76​ monkeypatch.setattr(file_utils, 'download', Dingus())
69 77​
70 self.helper.process(ref_input, self.ref_arg_data) 78​ self.helper.process(ref_input, self.ref_arg_data)
71 79​
72 download_calls = file_utils.download.calls 80​ download_calls = file_utils.download.calls
73 81​
74 assert len(download_calls) == 2 82​ assert len(download_calls) == 2
75 assert download_calls[0][1][0] == self._get_url(ref_path[0]) 83​ assert download_calls[0][1][0] == self._get_url(ref_path[0])
76 assert download_calls[1][1][0] == self._get_url(ref_path[1]) 84​ assert download_calls[1][1][0] == self._get_url(ref_path[1])
77 assert download_calls[0][1][2] == os.path.join(self.ref_arg_data['workdir'], 85​ assert download_calls[0][1][2] == os.path.join(self.ref_arg_data['workdir'],
78 ref_localpath[0]) 86​ ref_localpath[0])
79 assert download_calls[1][1][2] == os.path.join(self.ref_arg_data['workdir'], 87​ assert download_calls[1][1][2] == os.path.join(self.ref_arg_data['workdir'],
80 ref_localpath[1]) 88​ ref_localpath[1])
81 89​
82 def test_parse_download_command_incorrect_localpath(self): 90​ def test_parse_download_command_incorrect_localpath(self):
83 ref_path = ['file1', 'file2'] 91​ ref_path = ['file1', 'file2']
84 ref_localpath = ['file3'] 92​ ref_localpath = ['file3']
85 ref_input = {'action': 'download', 93​ ref_input = {'action': 'download',
86 'package': self.ref_nvr, 94​ 'package': self.ref_name,
95​ 'gitref': self.ref_branch,
87 'path': ref_path, 96​ 'path': ref_path,
88 'localpath': ref_localpath} 97​ 'localpath': ref_localpath}
89 98​
90 with pytest.raises(exc.TaskotronValueError): 99​ with pytest.raises(exc.TaskotronValueError):
91 self.helper.process(ref_input, self.ref_arg_data) 100​ self.helper.process(ref_input, self.ref_arg_data)
92 101​
93 def test_parse_download_command_incorrect_path_str(self): 102​ def test_parse_download_command_incorrect_path_str(self):
94 ref_path = 'file1' 103​ ref_path = 'file1'
95 ref_input = {'action': 'download', 104​ ref_input = {'action': 'download',
96 'package': self.ref_nvr, 105​ 'package': self.ref_name,
106​ 'gitref': self.ref_branch,
97 'path': ref_path} 107​ 'path': ref_path}
98 108​
99 with pytest.raises(exc.TaskotronValueError): 109​ with pytest.raises(exc.TaskotronValueError):
100 self.helper.process(ref_input, self.ref_arg_data) 110​ self.helper.process(ref_input, self.ref_arg_data)
101 111​
102 def test_parse_download_command_incorrect_localpath_str(self): 112​ def test_parse_download_command_incorrect_localpath_str(self):
103 ref_localpath = 'file2' 113​ ref_localpath = 'file2'
104 ref_input = {'action': 'download', 114​ ref_input = {'action': 'download',
105 'package': self.ref_nvr, 115​ 'package': self.ref_name,
116​ 'gitref': self.ref_branch,
106 'path': self.ref_path, 117​ 'path': self.ref_path,
107 'localpath': ref_localpath} 118​ 'localpath': ref_localpath}
108 119​
109 with pytest.raises(exc.TaskotronValueError): 120​ with pytest.raises(exc.TaskotronValueError):
110 self.helper.process(ref_input, self.ref_arg_data) 121​ self.helper.process(ref_input, self.ref_arg_data)
111 122​
112 def test_parse_download_command_missing_package(self): 123​ def test_parse_download_command_missing_package(self):
113 ref_input = {'action': 'download', 124​ ref_input = {'action': 'download',
114 'path': self.ref_path} 125​ 'path': self.ref_path}
115 126​
116 with pytest.raises(exc.TaskotronDirectiveError): 127​ with pytest.raises(exc.TaskotronDirectiveError):
117 self.helper.process(ref_input, self.ref_arg_data) 128​ self.helper.process(ref_input, self.ref_arg_data)
118 129​
119 def test_parse_download_command_missing_path(self): 130​ def test_parse_download_command_missing_path(self):
120 ref_input = {'action': 'download', 131​ ref_input = {'action': 'download',
121 'package': self.ref_nvr} 132​ 'package': self.ref_name,
133​ 'gitref': self.ref_branch}
122 134​
123 with pytest.raises(exc.TaskotronDirectiveError): 135​ with pytest.raises(exc.TaskotronDirectiveError):
124 self.helper.process(ref_input, self.ref_arg_data) 136​ self.helper.process(ref_input, self.ref_arg_data)