PR#216: Add item and type arguments, deprecate subject - waiverdb

waiverdb

#216 Add item and type arguments, deprecate subject

Merged 5 years ago by lholecek. Opened 5 years ago by lholecek.

lholecek/waiverdb cli-subject-item-type into master

Add item and type arguments, deprecate subject

Lukas Holecek • 5 years ago

3349cc7

docs/release-notes.rst

file modified

		`@@ -2,6 +2,13 @@`
		`Release Notes`
		`=============`

		`+ Next Release`
		`+ ============`
		`+`
		+ * The :program:`waiverdb-cli` utility accepts new options
		+ :option:`--subject-identifier` and :option:`--subject-type` which deprecate
		+ :option:`--subject` option.
		`+`
		`WaiverDB 0.11`
		`=============`

docs/waiverdb-cli.rst

file modified

+10 -1

		`@@ -28,7 +28,16 @@`

		`.. option:: -s, --subject TEXT`

		`- Specify one subject for a result to waive.`
		`+ Deprecated. Use --subject-identifier and --subject-type instead. Subject`
		`+ for a result to waive.`
		`+`
		`+ .. option:: -i, --subject-identifier TEXT`
		`+`
		`+ Subject identifier for a result to waive.`
		`+`
		`+ .. option:: -T, --subject-type TEXT`
		`+`
		`+ Subject type for a result to waive.`

		`.. option:: -t, --testcase TEXT`

tests/test_cli.py

file modified

+136 -42

		`@@ -90,7 +90,7 @@`
		`openid`
		`""")`
		`runner = CliRunner()`
		`- args = ['-C', p.strpath, '-s', '{"subject.test": "test", "s": "t"}', '-t', 'testcase',`
		`+ args = ['-C', p.strpath, '-i', 'test', '-T', 'compose', '-t', 'testcase',`
		`'-c', 'comment']`
		`result = runner.invoke(waiverdb_cli, args)`
		`assert result.exit_code == 1`
		`@@ -112,7 +112,7 @@`
		`args = ['-C', p.strpath, '-p', 'fedora-26', '-c', 'comment']`
		`result = runner.invoke(waiverdb_cli, args)`
		`assert result.exit_code == 1`
		`- assert result.output == 'Error: Please specify one subject\n'`
		`+ assert result.output == 'Error: Please specify subject-identifier\n'`


		`def test_no_testcase(tmpdir):`
		`@@ -127,7 +127,7 @@`
		`openid`
		`""")`
		`runner = CliRunner()`
		`- args = ['-C', p.strpath, '-p', 'fedora-26', '-s', 'subject', '-c', 'comment']`
		`+ args = ['-C', p.strpath, '-p', 'fedora-26', '-i', 'item', '-T', 'compose', '-c', 'comment']`
		`result = runner.invoke(waiverdb_cli, args)`
		`assert result.exit_code == 1`
		`assert result.output == 'Error: Please specify testcase\n'`
		`@@ -145,7 +145,7 @@`
		`openid`
		`""")`
		`runner = CliRunner()`
		`- args = ['-C', p.strpath, '-p', 'fedora-26', '-s', 'subject', '-t', 'testcase']`
		`+ args = ['-C', p.strpath, '-p', 'fedora-26', '-i', 'item', '-T', 'compose', '-t', 'testcase']`
		`result = runner.invoke(waiverdb_cli, args)`
		`assert result.exit_code == 1`
		`assert result.output == 'Error: Please specify comment\n'`
		`@@ -157,7 +157,7 @@`
		`pytest.importorskip('openidc_client')`
		`with patch('openidc_client.OpenIDCClient.send_request') as mock_oidc_req:`
		`mock_rv = Mock()`
		`- mock_rv.json.return_value = {`
		`+ mock_rv.json.return_value = [{`
		`"comment": "This is fine",`
		`"id": 15,`
		`"product_version": "Parrot",`
		`@@ -166,7 +166,7 @@`
		`"timestamp": "2017-010-16T17:42:04.209638",`
		`"username": "foo",`
		`"waived": True`
		`- }`
		`+ }]`
		`mock_oidc_req.return_value = mock_rv`
		`p = tmpdir.join('client.conf')`
		`p.write("""`
		`@@ -208,7 +208,7 @@`
		`pytest.importorskip('requests_gssapi')`
		`with patch('requests.request') as mock_request:`
		`mock_rv = Mock()`
		`- mock_rv.json.return_value = {`
		`+ mock_rv.json.return_value = [{`
		`"comment": "This is fine",`
		`"id": 15,`
		`"product_version": "Parrot",`
		`@@ -217,7 +217,7 @@`
		`"timestamp": "2017-010-16T17:42:04.209638",`
		`"username": "foo",`
		`"waived": True`
		`- }`
		`+ }]`
		`mock_request.return_value = mock_rv`
		`p = tmpdir.join('client.conf')`
		`p.write("""`
		`@@ -239,7 +239,7 @@`
		`def test_submit_waiver_with_id(tmpdir):`
		`with patch('requests.request') as mock_request:`
		`mock_rv = Mock()`
		`- mock_rv.json.return_value = {`
		`+ mock_rv.json.return_value = [{`
		`"comment": "This is fine",`
		`"data": {"item": ["htop-1.0-1.fc22"], "type": ["bodhi_update"]},`
		`"id": 15,`
		`@@ -248,7 +248,7 @@`
		`"timestamp": "2017-010-16T17:42:04.209638",`
		`"username": "foo",`
		`"waived": True`
		`- }`
		`+ }]`
		`mock_request.return_value = mock_rv`
		`p = tmpdir.join('client.conf')`
		`p.write("""`
		`@@ -259,7 +259,7 @@`
		`runner = CliRunner()`
		`args = ['-C', p.strpath, '-p', 'Parrot', '-r', '123',`
		`'-c', "This is fine"]`
		`- result = runner.invoke(waiverdb_cli, args)`
		`+ result = runner.invoke(waiverdb_cli, args, catch_exceptions=False)`
		`mock_request.assert_called()`
		`assert result.output == 'Created waiver 15 for result with id 123\n'`

		`@@ -267,15 +267,25 @@`
		`def test_submit_waiver_with_multiple_ids(tmpdir):`
		`with patch('requests.request') as mock_request:`
		`mock_rv = Mock()`
		`- mock_rv.json.return_value = {`
		`- "comment": "This is fine",`
		`- "data": {"item": ["htop-1.0-1.fc22"], "type": ["bodhi_update"]},`
		`- "id": 15,`
		`- "testcase": {"name": "test.testcase"},`
		`- "timestamp": "2017-010-16T17:42:04.209638",`
		`- "username": "foo",`
		`- "waived": True`
		`- }`
		`+ mock_rv.json.return_value = [`
		`+ {`
		`+ "comment": "This is fine",`
		`+ "data": {"item": ["htop-1.0-1.fc22"], "type": ["bodhi_update"]},`
		`+ "id": 15,`
		`+ "testcase": {"name": "test.testcase"},`
		`+ "timestamp": "2017-010-16T17:42:04.209638",`
		`+ "username": "foo",`
		`+ "waived": True`
		`+ }, {`
		`+ "comment": "This is fine",`
		`+ "data": {"item": ["htop-1.0-2.fc22"], "type": ["bodhi_update"]},`
		`+ "id": 16,`
		`+ "testcase": {"name": "test.testcase"},`
		`+ "timestamp": "2017-010-16T17:42:04.209638",`
		`+ "username": "foo",`
		`+ "waived": True`
		`+ }`
		`+ ]`
		`mock_request.return_value = mock_rv`
		`p = tmpdir.join('client.conf')`
		`p.write("""`
		`@@ -286,11 +296,11 @@`
		`runner = CliRunner()`
		`args = ['-C', p.strpath, '-p', 'Parrot', '-r', '123', '-r', '456',`
		`'-c', "This is fine"]`
		`- result = runner.invoke(waiverdb_cli, args)`
		`+ result = runner.invoke(waiverdb_cli, args, catch_exceptions=False)`
		`mock_request.assert_called()`

		`assert result.output == 'Created waiver 15 for result with id 123\n\`
		`- Created waiver 15 for result with id 456\n'`
		`+ Created waiver 16 for result with id 456\n'`


		`def test_malformed_submission_with_id_and_subject_and_testcase(tmpdir):`
		`@@ -303,14 +313,14 @@`
		`""")`
		`args = ['-C', p.strpath, '-p', 'Parrot', '-r', '123', '-s',`
		`'{"subject.test": "test", "s": "t"}', '-c', "This is fine"]`
		`- result = runner.invoke(waiverdb_cli, args)`
		`+ result = runner.invoke(waiverdb_cli, args, catch_exceptions=False)`
		`assert result.output == 'Error: Please specify result_id or subject/testcase. Not both\n'`


		`def test_submit_waiver_for_original_spec_nvr_result(tmpdir):`
		`with patch('requests.request') as mock_request:`
		`mock_rv = Mock()`
		`- mock_rv.json.return_value = {`
		`+ mock_rv.json.return_value = [{`
		`"comment": "This is fine",`
		`"original_spec_nvr": "test",`
		`"id": 15,`
		`@@ -319,7 +329,7 @@`
		`"timestamp": "2017-010-16T17:42:04.209638",`
		`"username": "foo",`
		`"waived": True`
		`- }`
		`+ }]`
		`mock_request.return_value = mock_rv`
		`p = tmpdir.join('client.conf')`
		`p.write("""`
		`@@ -335,18 +345,34 @@`
		`assert result.output == 'Created waiver 15 for result with id 123\n'`


		`- def test_create_waiver_no_product_version(tmpdir):`
		`+ def test_create_waiver_product_version_missing(tmpdir):`
		`+ p = tmpdir.join('client.conf')`
		`+ p.write("""`
		`+ [waiverdb]`
		`+ auth_method=dummy`
		`+ api_url=http://localhost:5004/api/v1.0`
		`+ koji_base_url=https://koji.fedoraproject.org/kojihub`
		`+ """)`
		`+ runner = CliRunner()`
		`+ args = ['-C', p.strpath, '-s', '{"type": "koji_build", "item": "this-will-not-work"}',`
		`+ '-t', 'test.testcase', '-c', "This is fine"]`
		`+ result = runner.invoke(waiverdb_cli, args)`
		`+ assert result.output == 'Error: Please specify product version using --product-version\n'`
		`+`
		`+`
		`+ def test_create_waiver_product_version_from_koji_build(tmpdir):`
		`with patch('requests.request') as mock_request:`
		`mock_rv = Mock()`
		`- mock_rv.json.return_value = {`
		`+ mock_rv.json.return_value = [{`
		`"comment": "This is fine",`
		`"id": 15,`
		`- "subject": {"type": "koji_build", "item": "setup-2.8.71-7.el7_4"},`
		`+ "subject_type": "koji_build",`
		`+ "subject_identifier": "setup-2.8.71-7.el7_4",`
		`"testcase": "test.testcase",`
		`"timestamp": "2017-010-16T17:42:04.209638",`
		`"username": "foo",`
		`"waived": True`
		`- }`
		`+ }]`
		`mock_request.return_value = mock_rv`
		`p = tmpdir.join('client.conf')`
		`p.write("""`
		`@@ -358,28 +384,48 @@`
		`runner = CliRunner()`
		`args = ['-C', p.strpath, '-s', '{"type": "koji_build", "item": "setup-2.8.71-7.el7_4"}',`
		`'-t', 'test.testcase', '-c', "This is fine"]`
		`- result = runner.invoke(waiverdb_cli, args)`
		`+ result = runner.invoke(waiverdb_cli, args, catch_exceptions=False)`
		`mock_request.assert_called()`
		`- assert result.output.startswith('Created waiver 15 for result with subject ')`
		`- assert result.output.endswith(' and testcase test.testcase\n')`
		`- assert any(['{"type": "koji_build", "item": "setup-2.8.71-7.el7_4"}' in result.output,`
		`- '{"item": "setup-2.8.71-7.el7_4", "type": "koji_build"}' in result.output])`
		`+ assert result.output == (`
		`+ 'Created waiver 15 for result with '`
		`+ 'subject type koji_build, identifier setup-2.8.71-7.el7_4 '`
		`+ 'and testcase test.testcase\n'`
		`+ )`

		`- args = ['-C', p.strpath, '-s', '{"type": "koji_build", "item": "this-will-not-work"}',`
		`- '-t', 'test.testcase', '-c', "This is fine"]`
		`- result = runner.invoke(waiverdb_cli, args)`
		`- assert result.output == 'Error: Please specify product version using --product-version\n'`

		`+ def test_create_waiver_product_version_from_compose(tmpdir):`
		`+ with patch('requests.request') as mock_request:`
		`+ mock_rv = Mock()`
		`+ mock_rv.json.return_value = [{`
		`+ "comment": "This is fine",`
		`+ "id": 15,`
		`+ "subject_type": "compose",`
		`+ "subject_identifier": "Fedora-Rawhide-20180526.n.1",`
		`+ "testcase": "test.testcase",`
		`+ "timestamp": "2017-010-16T17:42:04.209638",`
		`+ "username": "foo",`
		`+ "waived": True`
		`+ }]`
		`+ mock_request.return_value = mock_rv`
		`+ p = tmpdir.join('client.conf')`
		`+ p.write("""`
		`+ [waiverdb]`
		`+ auth_method=dummy`
		`+ api_url=http://localhost:5004/api/v1.0`
		`+ koji_base_url=https://koji.fedoraproject.org/kojihub`
		`+ """)`
		`+ runner = CliRunner()`
		`args = ['-C', p.strpath, '-s', ('{"productmd.compose.id": "Fedora-Rawhide-20180526.n.1",'`
		`'"type": "compose",'`
		`'"item": "Fedora-Rawhide-20180526.n.1"}'),`
		`'-t', 'test.testcase', '-c', "This is fine"]`
		`result = runner.invoke(waiverdb_cli, args)`
		`mock_request.assert_called()`
		`- assert result.output == ('Created waiver 15 for result with subject '`
		`- '{"productmd.compose.id": "Fedora-Rawhide-20180526.n.1", "type": '`
		`- '"compose", "item": "Fedora-Rawhide-20180526.n.1"} and testcase '`
		`- 'test.testcase\n')`
		`+ assert result.output == (`
		`+ 'Created waiver 15 for result with '`
		`+ 'subject type compose, identifier Fedora-Rawhide-20180526.n.1 '`
		`+ 'and testcase test.testcase\n'`
		`+ )`


		`def test_guess_product_version():`
		`@@ -389,3 +435,51 @@`
		`assert guess_product_version('Fedora-28-20180423.n.0') == 'fedora-28'`
		`assert guess_product_version('Fedora-Rawhide-20180524.n.0') == 'fedora-rawhide'`
		`assert guess_product_version('Fedora-Atomic-28-20180424.4') is None`
		`+`
		`+`
		`+ def test_create_waiver_missing_subject_identifier(tmpdir):`
		`+ p = tmpdir.join('client.conf')`
		`+ p.write("""`
		`+ [waiverdb]`
		`+ auth_method=dummy`
		`+ api_url=http://localhost:5004/api/v1.0`
		`+ koji_base_url=https://koji.fedoraproject.org/kojihub`
		`+ """)`
		`+ runner = CliRunner()`
		`+ args = ['-C', p.strpath, '-T', 'koji_build',`
		`+ '-t', 'test.testcase', '-c', "This is fine"]`
		`+ result = runner.invoke(waiverdb_cli, args)`
		`+ assert result.exit_code != 0`
		`+ assert 'Error: Please specify subject-identifier' in result.output`
		`+`
		`+`
		`+ def test_create_waiver_missing_subject_type(tmpdir):`
		`+ p = tmpdir.join('client.conf')`
		`+ p.write("""`
		`+ [waiverdb]`
		`+ auth_method=dummy`
		`+ api_url=http://localhost:5004/api/v1.0`
		`+ koji_base_url=https://koji.fedoraproject.org/kojihub`
		`+ """)`
		`+ runner = CliRunner()`
		`+ args = ['-C', p.strpath, '-i', 'setup-2.8.71-7.el7_4',`
		`+ '-t', 'test.testcase', '-c', "This is fine"]`
		`+ result = runner.invoke(waiverdb_cli, args)`
		`+ assert result.exit_code != 0`
		`+ assert 'Error: Please specify correct subject type' in result.output`
		`+`
		`+`
		`+ def test_create_waiver_invalid_subject_type(tmpdir):`
		`+ p = tmpdir.join('client.conf')`
		`+ p.write("""`
		`+ [waiverdb]`
		`+ auth_method=dummy`
		`+ api_url=http://localhost:5004/api/v1.0`
		`+ koji_base_url=https://koji.fedoraproject.org/kojihub`
		`+ """)`
		`+ runner = CliRunner()`
		`+ args = ['-C', p.strpath, '-T', 'brew-build', '-i', 'setup-2.8.71-7.el7_4',`
		`+ '-t', 'test.testcase', '-c', "This is fine"]`
		`+ result = runner.invoke(waiverdb_cli, args)`
		`+ assert result.exit_code != 0`
		`+ assert 'invalid choice: brew-build' in result.output`

waiverdb/api_v1.py

file modified

+2 -1

		`@@ -280,7 +280,8 @@`
		`"comment": "This is fine",`
		`"id": 15,`
		`"product_version": "Parrot",`
		`- "subject": {"productmd.compose.id": "Fedora-9000-19700101.n.18"},`
		`+ "subject_type": "compose",`
		`+ "subject_identifier": "Fedora-9000-19700101.n.18",`
		`"testcase": "compose.install_no_user",`
		`"timestamp": "2017-03-16T17:42:04.209638",`
		`"username": "jcline",`

waiverdb/cli.py

file modified

+109 -49

		`@@ -11,6 +11,30 @@`

		`requests_session = requests.Session()`

		`+ SUBJECT_TYPES = ("koji_build", "bodhi_update", "compose")`
		`+`
		`+`
		`+ class OldJSONSubject(click.ParamType):`
		`+ """`
		`+ Deprecated JSON subject CLI parameter.`
		`+ """`
		`+ name = 'Deprecated JSON subject'`
		`+`
		`+ def convert(self, value, param, ctx):`
		`+ if not isinstance(value, str):`
		`+ return value`
		`+`
		`+ try:`
		`+ subject = json.loads(value)`
		`+ except json.JSONDecodeError as e:`
		`+ raise click.ClickException('Invalid JSON object: {}'.format(e))`
		`+`
		`+ if not isinstance(subject, dict):`
		`+ raise click.ClickException(`
		`+ 'Failed to parse JSON. Please use id and type instead of using subject.')`
		`+`
		`+ return subject`
		`+`

		`def validate_config(config):`
		`"""`
		`@@ -35,22 +59,36 @@`
		`raise click.ClickException(config_error.format(required_config))`


		`- def check_response(resp, data, result_id=None):`
		`- if result_id:`
		`- msg = 'for result with id {0}'.format(result_id)`
		`- else:`
		`- msg = 'for result with subject {0} and testcase {1}'.format(json.dumps(data['subject']),`
		`- data['testcase'])`
		`+ def print_result(waiver_id, result_description):`
		`+ click.echo('Created waiver {0} for result with {1}'.format(waiver_id, result_description))`
		`+`
		`+`
		`+ def check_response(resp, result_ids):`
		`if not resp.ok:`
		`try:`
		`error_msg = resp.json()['message']`
		`except (ValueError, KeyError):`
		`error_msg = resp.text`
		`raise click.ClickException(`
		`- 'Failed to create waiver {0}:\n{1}'`
		`- .format(msg, error_msg))`
		`- click.echo('Created waiver {0} {1}'.format(`
		`- resp.json()['id'], msg))`
		`+ 'Failed to create waivers: {0}'`
		`+ .format(error_msg))`
		`+`
		`+ response_data = resp.json()`
		`+ if result_ids:`
		`+ if len(response_data) != len(result_ids):`
		`+ raise RuntimeError(`
		`+ 'Unexpected number of results in response: {!r}'.format(response_data))`
		`+`
		`+ for result_id, data in zip(result_ids, response_data):`
		`+ waiver_id = data['id']`
		`+ msg = 'id {0}'.format(result_id)`
		`+ print_result(waiver_id, msg)`
		`+ else:`
		`+ for data in response_data:`
		`+ waiver_id = data['id']`
		`+ msg = 'subject type {0}, identifier {1} and testcase {2}'.format(`
		`+ data['subject_type'], data['subject_identifier'], data['testcase'])`
		`+ print_result(waiver_id, msg)`


		`def guess_product_version(toparse, koji_build=False):`
		`@@ -85,8 +123,13 @@`
		`help='Specify a config file to use')`
		`@click.option('--result-id', '-r', multiple=True, type=int,`
		`help='Specify one or more results to be waived')`
		`- @click.option('--subject', '-s',`
		`- help='Specify one subject for a result to waive')`
		`+ @click.option('--subject', '-s', type=OldJSONSubject(),`
		`+ help=('Deprecated. Use --subject-identifier and --subject-type instead. '`
		`+ 'Subject for a result to waive.'))`
		`+ @click.option('--subject-identifier', '-i',`
		`+ help='Subject identifier for a result to waive.')`
		`+ @click.option('--subject-type', '-T', type=click.Choice(SUBJECT_TYPES),`
		`+ help='Subject type for a result to waive.')`
		`@click.option('--testcase', '-t',`
		`help='Specify a testcase for the subject')`
		`@click.option('--product-version', '-p',`
		`@@ -95,7 +138,8 @@`
		`help='Whether or not the result is waived')`
		`@click.option('--comment', '-c',`
		`help='A comment explaining why the result is waived')`
		`- def cli(comment, waived, product_version, testcase, subject, result_id, config_file):`
		`+ def cli(comment, waived, product_version, testcase, subject, subject_identifier, subject_type,`
		`+ result_id, config_file):`
		`"""`
		`Creates new waiver against test results.`

		`@@ -105,8 +149,7 @@`
		`waiverdb-cli -r 47 -r 48 -p "fedora-28" -c "This is fine"`

		`\b`
		`- waiverdb-cli -t dist.rpmdeplint \\`
		`- -s '{"item": "qclib-1.3.1-3.fc28", "type": "koji_build"}' \\`
		`+ waiverdb-cli -t dist.rpmdeplint -i qclib-1.3.1-3.fc28 -T koji_build \\`
		`-p "fedora-28" -c "This is expected for non-x86 packages"`
		`"""`
		`config = configparser.SafeConfigParser()`
		`@@ -115,20 +158,39 @@`
		`validate_config(config)`

		`result_ids = result_id`
		`+`
		`+ # Backward compatibility with v0.11.0.`
		`+ if subject:`
		`+ if subject_identifier or subject_type:`
		`+ raise click.ClickException(`
		`+ 'Please don\'t specify subject when using identifier and type.')`
		`+ if result_ids:`
		`+ raise click.ClickException('Please specify result_id or subject/testcase. Not both')`
		`+`
		`+ subject_identifier = subject.get("productmd.compose.id")`
		`+ if subject_identifier:`
		`+ subject_type = "compose"`
		`+ else:`
		`+ subject_identifier = subject.get("item")`
		`+ subject_type = subject.get("type")`
		`+`
		`if not comment:`
		`raise click.ClickException('Please specify comment')`
		`- if result_ids and (subject or testcase):`
		`- raise click.ClickException('Please specify result_id or subject/testcase. Not both')`
		`- if not result_ids and not subject:`
		`- raise click.ClickException('Please specify one subject')`
		`+ if result_ids and (testcase or subject_identifier):`
		`+ raise click.ClickException('Please specify result_id or id/type/testcase. Not both')`
		`+ if not result_ids and not subject_identifier:`
		`+ raise click.ClickException('Please specify subject-identifier')`
		`if not result_ids and not testcase:`
		`raise click.ClickException('Please specify testcase')`
		`+ if not result_ids and subject_type not in SUBJECT_TYPES:`
		`+ raise click.ClickException(`
		`+ 'Please specify correct subject type {!r}.'.format(SUBJECT_TYPES))`

		`if not product_version and not result_ids:`
		`# trying to guess the product_version`
		`- if json.loads(subject).get('type', None) == 'koji_build':`
		`+ if subject_type == 'koji_build':`
		`try:`
		`- short_prod_version = json.loads(subject)['item'].split('.')[-1]`
		`+ short_prod_version = subject_identifier.split('.')[-1]`
		`product_version = guess_product_version(short_prod_version, koji_build=True)`
		`except KeyError:`
		`pass`
		`@@ -138,7 +200,7 @@`
		`koji_base_url = config.get('waiverdb', 'koji_base_url')`
		`proxy = client.ServerProxy(koji_base_url)`
		`try:`
		`- build = proxy.getBuild(json.loads(subject)['item'])`
		`+ build = proxy.getBuild(subject_identifier)`
		`if build:`
		`target = proxy.getTaskRequest(build['task_id'])[1]`
		`product_version = guess_product_version(target, koji_build=True)`
		`@@ -147,9 +209,8 @@`
		`except client.Fault:`
		`pass`

		`- if "productmd.compose.id" in json.loads(subject):`
		`- product_version = guess_product_version(`
		`- json.loads(subject)["productmd.compose.id"])`
		`+ if subject_type == "compose":`
		`+ product_version = guess_product_version(subject_identifier)`

		`if not product_version:`
		`raise click.ClickException('Please specify product version using --product-version')`
		`@@ -158,7 +219,8 @@`
		`data_list = []`
		`if not result_ids:`
		`data_list.append({`
		`- 'subject': json.loads(subject),`
		`+ 'subject_identifier': subject_identifier,`
		`+ 'subject_type': subject_type,`
		`'testcase': testcase,`
		`'waived': waived,`
		`'product_version': product_version,`
		`@@ -175,6 +237,13 @@`
		`})`

		`api_url = config.get('waiverdb', 'api_url')`
		`+ url = '{0}/waivers/'.format(api_url.rstrip('/'))`
		`+ data = json.dumps(data_list)`
		`+ common_request_arguments = {`
		`+ 'data': data,`
		`+ 'headers': {'Content-Type': 'application/json'},`
		`+ 'timeout': 60,`
		`+ }`
		`if auth_method == 'OIDC':`
		`# Try to import this now so the user gets immediate feedback if`
		`# it isn't installed`
		`@@ -194,14 +263,11 @@`
		`oidc_client_secret)`
		`scopes = config.get('waiverdb', 'oidc_scopes').strip().splitlines()`

		`- for data in data_list:`
		`- resp = oidc.send_request(`
		`- scopes=scopes,`
		`- url='{0}/waivers/'.format(api_url.rstrip('/')),`
		`- data=json.dumps(data),`
		`- headers={'Content-Type': 'application/json'},`
		`- timeout=60)`
		`- check_response(resp, data, data.get('result_id', None))`
		`+ resp = oidc.send_request(`
		`+ scopes=scopes,`
		`+ url=url,`
		`+ **common_request_arguments)`
		`+ check_response(resp, result_ids)`
		`elif auth_method == 'Kerberos':`
		`# Try to import this now so the user gets immediate feedback if`
		`# it isn't installed`
		`@@ -212,22 +278,16 @@`
		`'python-requests-gssapi needs to be installed')`
		`auth = requests_gssapi.HTTPKerberosAuth(`
		`mutual_authentication=requests_gssapi.OPTIONAL)`
		`- for data in data_list:`
		`- resp = requests.request('POST', '{0}/waivers/'.format(api_url.rstrip('/')),`
		`- data=json.dumps(data), auth=auth,`
		`- headers={'Content-Type': 'application/json'},`
		`- timeout=60)`
		`- if resp.status_code == 401:`
		`- raise click.ClickException('WaiverDB authentication using GSSAPI failed. '`
		`- 'Make sure you have a valid Kerberos ticket.')`
		`- check_response(resp, data, data.get('result_id', None))`
		`+ resp = requests.request(`
		`+ 'POST', url, auth=auth, **common_request_arguments)`
		`+ if resp.status_code == 401:`
		`+ raise click.ClickException('WaiverDB authentication using GSSAPI failed. '`
		`+ 'Make sure you have a valid Kerberos ticket.')`
		`+ check_response(resp, result_ids)`
		`elif auth_method == 'dummy':`
		`- for data in data_list:`
		`- resp = requests.request('POST', '{0}/waivers/'.format(api_url.rstrip('/')),`
		`- data=json.dumps(data), auth=('user', 'pass'),`
		`- headers={'Content-Type': 'application/json'},`
		`- timeout=60)`
		`- check_response(resp, data, data.get('result_id', None))`
		`+ resp = requests.request(`
		`+ 'POST', url, auth=('user', 'pass'), **common_request_arguments)`
		`+ check_response(resp, result_ids)`


		`if __name__ == '__main__':`

lholecek commented 5 years ago

Fixes #213

gnaponie commented on line 144 of waiverdb/cli.py 5 years ago

We wanted to deprecate the result-id as a parameter, it is there only for backwards compatibility so I think we shouldn't advice to provide a result-id. I would just advice to use the subject_identifier. Also is it confusing to call it id_?

gnaponie commented 5 years ago

beside that comment it looks ok to me

lholecek commented on line 144 of waiverdb/cli.py 5 years ago

OK, I fixed the error message and renamed the new arguments to --subject-identifier and --subject-type.

rebased onto 3349cc7

5 years ago

gnaponie commented 5 years ago