PR#389: Add the ability to pass in buildrequire and require overrides on a module build - rpkg

rpkg

#389 Add the ability to pass in buildrequire and require overrides on a module build

Merged 5 years ago by lsedlar. Opened 5 years ago by mprahl.

Unknown source mbs-dep-overrides into master

Add the ability to pass in buildrequire and require overrides on a module build

mprahl • 5 years ago

89e34eb

pyrpkg/__init__.py

file modified

+19 -3

		`@@ -3478,9 +3478,9 @@`
		`return resp`

		`def module_submit_build(self, scm_url, branch, auth_method,`
		`- optional=None, oidc_id_provider=None,`
		`- oidc_client_id=None, oidc_client_secret=None,`
		`- oidc_scopes=None):`
		`+ buildrequires=None, requires=None, optional=None,`
		`+ oidc_id_provider=None, oidc_client_id=None,`
		`+ oidc_client_secret=None, oidc_scopes=None):`
		`"""`
		`Submit a module build to the MBS`

		`@@ -3488,6 +3488,10 @@`
		`:param branch: a string of the module's branch`
		`:param str auth_method: a string of the authentication method used by`
		`the MBS.`
		`+ :param list buildrequires: a list of buildrequires in the format of`
		`+ 'name:stream' to override.`
		`+ :param list requires: a list of requires in the format of 'name:stream'`
		`+ to override.`
		`:param optional: an optional list of "key=value" to be passed in with`
		`the MBS build submission.`
		`:type optional: list[str]`
		`@@ -3514,10 +3518,22 @@`
		`raise rpkgError(`
		`'Optional arguments are not in the proper "key=value" format')`

		`+ for dep_type, overrides in (('buildrequires', buildrequires),`
		`+ ('requires', requires)):`
		`+ for override in overrides or []:`
		`+ dep_name, dep_stream = override`
		`+ # Get the override key that MBS accepts (e.g.`
		`+ # buildrequire_overrides)`
		`+ key = '{0}_overrides'.format(dep_type[:-1])`
		`+ body.setdefault(key, {})`
		`+ body[key].setdefault(dep_name, [])`
		`+ body[key][dep_name].append(dep_stream)`
		`+`
		`conflicting_keys = set(body.keys()) & set(optional_dict.keys())`
		`if conflicting_keys:`
		`raise rpkgError('The following optional arguments conflict with other arguments: {0}'`
		`.format(', '.join(conflicting_keys)))`
		`+`
		`body.update(optional_dict)`
		`url = self.module_get_url(build_id=None, action='POST')`
		`resp = self.module_send_authorized_request(`

pyrpkg/cli.py

file modified

+11 -2

		`@@ -1082,6 +1082,14 @@`
		`'--watch', '-w', help='Watch the module build',`
		`action='store_true')`
		`self.module_build_parser.add_argument(`
		`+ '--buildrequires', action='append', metavar='name:stream',`
		`+ dest='buildrequires', type=utils.validate_module_dep_override,`
		`+ help='Buildrequires to override in the form of "name:stream"')`
		`+ self.module_build_parser.add_argument(`
		`+ '--requires', action='append', metavar='name:stream',`
		`+ dest='requires', type=utils.validate_module_dep_override,`
		`+ help='Requires to override in the form of "name:stream"')`
		`+ self.module_build_parser.add_argument(`
		`'--optional', action='append', metavar='KEY=VALUE',`
		`dest='optional',`
		`help='MBS optional arguments in the form of "key=value"')`
		`@@ -1972,8 +1980,9 @@`
		`if not self.args.q:`
		`print('Submitting the module build...')`
		`build_ids = self._cmd.module_submit_build(`
		`- scm_url, branch, auth_method, self.args.optional,`
		`- oidc_id_provider, oidc_client_id, oidc_client_secret, oidc_scopes)`
		`+ scm_url, branch, auth_method, self.args.buildrequires,`
		`+ self.args.requires, self.args.optional, oidc_id_provider,`
		`+ oidc_client_id, oidc_client_secret, oidc_scopes)`
		`if self.args.watch:`
		`self.module_watch_build(build_ids)`
		`elif not self.args.q:`

pyrpkg/utils.py

file modified

		`@@ -105,3 +105,11 @@`
		`cmd += ['--config', args.config]`

		`return cmd`
		`+`
		`+`
		`+ def validate_module_dep_override(dep):`
		`+ """Validate the passed-in module dependency override."""`
		`+ try:`
		`+ return dep.split(':', 1)`
		`+ except (ValueError, AttributeError):`
		`+ raise argparse.ArgumentTypeError('This option must be in the format of "name:stream"')`

tests/test_cli.py

file modified

+33

		`@@ -2073,6 +2073,39 @@`
		`self.assertEqual(output, expected_output)`

		`@patch('requests.get')`
		`+ @patch('openidc_client.OpenIDCClient.send_request')`
		`+ def test_module_build_dep_overrides(self, mock_oidc_req, mock_get):`
		`+ """`
		`+ Test a module build with buildrequire and require overrides`
		`+ """`
		`+ cli_cmd = [`
		`+ 'rpkg',`
		`+ '--path', self.cloned_repo_path,`
		`+ 'module-build',`
		`+ 'git://pkgs.fedoraproject.org/modules/testmodule?#79d87a5a',`
		`+ 'master',`
		`+ '--buildrequires', 'platform:f28',`
		`+ '--buildrequires', 'platform:f29',`
		`+ '--requires', 'platform:f29'`
		`+ ]`
		`+ mock_get.return_value.ok = True`
		`+ mock_get.return_value.json.return_value = {`
		`+ 'auth_method': 'oidc',`
		`+ 'api_version': 2`
		`+ }`
		`+ mock_oidc_req.return_value.json.return_value = [`
		`+ {'id': 1094}, {'id': 1095}, {'id': 1096}]`
		`+`
		`+ with patch('sys.argv', new=cli_cmd):`
		`+ cli = self.new_cli()`
		`+ cli.module_build()`
		`+`
		`+ brs = mock_oidc_req.call_args[1]['json']['buildrequire_overrides']`
		`+ assert set(brs['platform']) == set(['f28', 'f29'])`
		`+ reqs = mock_oidc_req.call_args[1]['json']['require_overrides']`
		`+ assert reqs['platform'] == ['f29']`
		`+`
		`+ @patch('requests.get')`
		`def test_module_build_conflicting_keys(self, mock_get):`
		`"""`
		`Test a module build with optional arguments the conflict with other arguments`

mprahl commented 5 years ago

Addresses part of FACTORY-3084

mprahl commented 5 years ago

@cqi @jkaluza @ralph could you please review?

rebased onto f116860c6195e5234a3d872f6ba3db8c7a835de5

5 years ago

rebased onto be9e9f2da80da933e03a254179284bd8d7bf8f1e

5 years ago

rebased onto 0e72a3f6b3e6b6e0951b2d82663c601801f655f0

5 years ago

rebased onto c40dbb23211c117d957776f0ac72dbfe85a6cd21

5 years ago

ralph commented 5 years ago

:+1:

lucarval commented on line 47 of pyrpkg/__init__.py 5 years ago

It might be a good idea to disallow certain keys in optional_dict. For example, if optional parameter contains any of these it would cause a confusing behavior: scmurl, branch, buildrequire_overrides, require_overrides.

mprahl commented on line 47 of pyrpkg/__init__.py 5 years ago

Good idea. I should probably make this a separate PR though since this problem existed before this PR.

mprahl commented on line 47 of pyrpkg/__init__.py 5 years ago

@lucarval, I submitted a PR that should address this:
https://pagure.io/rpkg/pull-request/391

lucarval commented 5 years ago

:+1:

mprahl commented 5 years ago

@lsedlar could you please review this as well?

cqi commented 5 years ago

:thumbsup:

A minor suggestion is to validate --buildrequires and --requires values during argparse parses option values. Setting a validation function to argument type parameter.

lsedlar commented on line 10 of pyrpkg/__init__.py 5 years ago

FYI adding kwargs not at the end is not backwards compatible and would break users that call the method without naming the kwargs. But hey, that does not happen here, so +1 for this more reasonable ordering.

lsedlar commented 5 years ago

Looks good to me. :thumbsup:

rebased onto 5a054d769be4ebf917e5430d0c440ab449c8738b

5 years ago

rebased onto 93720cbe75b217de25c8ff5546e4f9cd8f855d68

5 years ago

rebased onto 89e34eb

5 years ago