PR#2493: getBuildConfig returns inheritance history - koji

		`@@ -4866,10 +4866,10 @@`

		`tags = []`
		`for tag in args:`
		`- info = session.getTag(tag, **event_opts)`
		`+ info = session.getBuildConfig(tag, **event_opts)`
		`if info is None:`
		`try:`
		`- info = session.getTag(int(tag), **event_opts)`
		`+ info = session.getBuildConfig(int(tag), **event_opts)`
		`except ValueError:`
		`info = None`
		`if info is None:`
		`@@ -4896,7 +4896,10 @@`
		`if 'extra' in info:`
		`print("Tag options:")`
		`for key in sorted(info['extra'].keys()):`
		`- print(" %s : %s" % (key, pprint.pformat(info['extra'][key])))`
		`+ line = " %s : %s" % (key, pprint.pformat(info['extra'][key]))`
		`+ if key in info.get('config_inheritance', {}).get('extra', []):`
		`+ line = "%-30s [%s]" % (line, info['config_inheritance']['extra'][key]['name'])`
		`+ print(line)`
		`dest_targets = session.getBuildTargets(destTagID=info['id'], **event_opts)`
		`build_targets = session.getBuildTargets(buildTagID=info['id'], **event_opts)`
		`repos = {}`

hub/kojihub.py

file modified

		`@@ -11949,6 +11949,7 @@`
		`def getBuildConfig(self, tag, event=None):`
		`"""Return build configuration associated with a tag"""`
		`taginfo = get_tag(tag, strict=True, event=event, blocked=True)`
		`+ taginfo['config_inheritance'] = {'extra': {}, 'arches': None}`
		`order = readFullInheritance(taginfo['id'], event=event)`
		`# follow inheritance for arches and extra`
		`for link in order:`
		`@@ -11957,9 +11958,11 @@`
		`ancestor = get_tag(link['parent_id'], strict=True, event=event, blocked=True)`
		`if taginfo['arches'] is None and ancestor['arches'] is not None:`
		`taginfo['arches'] = ancestor['arches']`
		`+ taginfo['config_inheritance']['arches'] = dslice(ancestor, ('id', 'name'))`
		`for key in ancestor['extra']:`
		`if key not in taginfo['extra']:`
		`taginfo['extra'][key] = ancestor['extra'][key]`
		`+ taginfo['config_inheritance']['extra'][key] = dslice(ancestor, ('id', 'name'))`
		`# cleanup extras by blocked`
		`for k, v in list(taginfo['extra'].items()):`
		`if v[0]:`

tests/test_hub/test_get_build_config.py

file added

+126

		`@@ -0,0 +1,126 @@`
		`+ import mock`
		`+ import unittest`
		`+`
		`+ import kojihub`
		`+`
		`+`
		`+ class TestGetBuildConfig(unittest.TestCase):`
		`+`
		`+ @mock.patch('kojihub.readFullInheritance')`
		`+ @mock.patch('kojihub.get_tag')`
		`+ def test_simple_tag(self, get_tag, readFullInheritance):`
		`+ tag = 'tag_name'`
		`+ get_tag.return_value = {'id': 123, 'name': tag, 'extra': {}}`
		`+ readFullInheritance.return_value = []`
		`+`
		`+ taginfo = kojihub.RootExports().getBuildConfig(tag)`
		`+`
		`+ get_tag.assert_called_with(tag, event=None, strict=True)`
		`+ readFullInheritance.assert_called_with(123, event=None)`
		`+`
		`+ self.assertEqual(taginfo, {`
		`+ 'id': 123,`
		`+ 'name': tag,`
		`+ 'extra': {},`
		`+ 'config_inheritance': {'extra': {}, 'arches': None},`
		`+ })`
		`+`
		`+ @mock.patch('kojihub.readFullInheritance')`
		`+ @mock.patch('kojihub.get_tag')`
		`+ def test_basic_inherited(self, get_tag, readFullInheritance):`
		`+ tag = 'tag_name'`
		`+ get_tag.side_effect = [`
		`+ {`
		`+ 'id': 123,`
		`+ 'name': tag,`
		`+ 'extra': {},`
		`+ 'arches': None,`
		`+ },`
		`+ {`
		`+ 'id': 1234,`
		`+ 'name': 'parent',`
		`+ 'extra': {'value': 'inherited'},`
		`+ 'arches': 'x86_64',`
		`+ },`
		`+ ]`
		`+ readFullInheritance.return_value = [`
		`+ {`
		`+ 'child_id': 123,`
		`+ 'currdepth': 1,`
		`+ 'filter': [],`
		`+ 'intransitive': False,`
		`+ 'maxdepth': None,`
		`+ 'name': tag,`
		`+ 'nextdepth': None,`
		`+ 'noconfig': False,`
		`+ 'parent_id': 1234,`
		`+ 'pkg_filter': '',`
		`+ 'priority': 0`
		`+ }`
		`+ ]`
		`+`
		`+ taginfo = kojihub.RootExports().getBuildConfig(tag, event=1111)`
		`+`
		`+ get_tag.assert_has_calls([`
		`+ mock.call(tag, event=1111, strict=True),`
		`+ mock.call(1234, event=1111, strict=True),`
		`+ ])`
		`+ readFullInheritance.assert_called_with(123, event=1111)`
		`+`
		`+ self.assertEqual(taginfo, {`
		`+ 'arches': 'x86_64',`
		`+ 'extra': {'value': 'inherited'},`
		`+ 'config_inheritance': {`
		`+ 'arches': {'id': 1234, 'name': 'parent'},`
		`+ 'extra' : {'value': {'id': 1234, 'name': 'parent'}}`
		`+ },`
		`+ 'id': 123,`
		`+ 'name': 'tag_name'`
		`+ })`
		`+`
		`+ @mock.patch('kojihub.readFullInheritance')`
		`+ @mock.patch('kojihub.get_tag')`
		`+ def test_inherited_noconfig(self, get_tag, readFullInheritance):`
		`+ tag = 'tag_name'`
		`+ get_tag.side_effect = [`
		`+ {`
		`+ 'id': 123,`
		`+ 'name': tag,`
		`+ 'extra': {},`
		`+ 'arches': None,`
		`+ },`
		`+ {`
		`+ 'id': 1234,`
		`+ 'name': 'parent',`
		`+ 'extra': {'value': 'inherited'},`
		`+ 'arches': 'x86_64',`
		`+ },`
		`+ ]`
		`+ readFullInheritance.return_value = [`
		`+ {`
		`+ 'child_id': 123,`
		`+ 'currdepth': 1,`
		`+ 'filter': [],`
		`+ 'intransitive': False,`
		`+ 'maxdepth': None,`
		`+ 'name': tag,`
		`+ 'nextdepth': None,`
		`+ 'noconfig': True,`
		`+ 'parent_id': 1234,`
		`+ 'pkg_filter': '',`
		`+ 'priority': 0`
		`+ }`
		`+ ]`
		`+`
		`+ taginfo = kojihub.RootExports().getBuildConfig(tag, event=1111)`
		`+`
		`+ get_tag.assert_called_once_with(tag, event=1111, strict=True)`
		`+ readFullInheritance.assert_called_with(123, event=1111)`
		`+`
		`+ self.assertEqual(taginfo, {`
		`+ 'arches': None,`
		`+ 'extra': {},`
		`+ 'config_inheritance': {'extra': {}, 'arches': None},`
		`+ 'id': 123,`
		`+ 'name': 'tag_name'`
		`+ })`

tkopecek commented 2 years ago

%SUBJ% + taginfo shows inherited extra

tkopecek commented 2 years ago

@mikem @julian8628 I've extended getBuildConfig to return also basic inheritance data (without full chain. It is for free (only sending a bit more data over the wire).

If you see it usable as next step I would extend getBuild(taginfo, event, strict) to getBuild(taginfo, event, strict, inherit) and make getBuildConfig alias to this? (separate issue)

tkopecek commented 2 years ago

If merged - I would use it in #2472 to get inherited extra.

tkopecek commented 2 years ago

related to #2495

mikem commented 2 years ago

Fails if used against old hub:

Traceback (most recent call last):
  File "/home/mikem/Devel/koji/koji/cli2/lkoji", line 337, in <module>
    rv = locals()[command].__call__(options, session, args)
  File "/home/mikem/Devel/koji/koji/cli/koji_cli/commands.py", line 4888, in anon_handle_taginfo
    if key in info['extra_inheritance']:
KeyError: 'extra_inheritance'

mikem commented 2 years ago

Minor:

[mikem@localhost koji]$ git diff origin... |flake8 --diff
tests/test_hub/test_get_build_config.py:6:1: E302 expected 2 blank lines, found 1

mikem commented 2 years ago

Other than that, the change looks fine

2 new commits added

flake8 fix
fix backward compatibility

2 years ago

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

2 years ago

julian8628 commented 2 years ago

:thumbsup:

julian8628 commented 2 years ago

It seems we could show the inheritance for arches as well.
Does it make sense to do this in the output of taginfo?

1 new commit added

extend to config_inheritance

2 years ago

tkopecek commented 2 years ago

I've changed it to more broad config_inheritance, but not exposing it anywhere yet.

Metadata Update from @mfilip:
- Pull-request tagged with: testing-done

2 years ago

rebased onto 172c861

2 years ago

tkopecek commented 2 years ago

Commit bfd3034 fixes this pull-request

Pull-Request has been merged by tkopecek

2 years ago

Metadata

Assignee

None

Tags

Flags

jenkins

success (100%)

Build #2227 successful (commit: 5cb7db8e)

2 years ago

jenkins

success (100%)

Build #2203 successful (commit: e4a760c7)

2 years ago

jenkins

success (100%)

Build #2194 successful (commit: f8e5946b)

2 years ago

Changes Summary 3

+6 -3

file changed

cli/koji_cli/commands.py

file added

tests/test_hub/test_get_build_config.py

koji

Source Code

Documentation

#2493 getBuildConfig returns inheritance history Merged 2 years ago by tkopecek. Opened 2 years ago by tkopecek. tkopecek/koji issue1870 into master

Metadata

testing-ready testing-done

Flags

Changes Summary 3

#2493 getBuildConfig returns inheritance history

Merged 2 years ago by tkopecek. Opened 2 years ago by tkopecek.

tkopecek/koji issue1870 into master