PR#1204: config: Deprecate release_is_layered option - pungi

pungi

#1204 config: Deprecate release_is_layered option

Merged 4 years ago by lsedlar. Opened 4 years ago by hlin.

hlin/pungi deprecate-release_is_layered-option into master

config: Deprecate release_is_layered option

Haibo Lin • 4 years ago

1de824d

bin/pungi-koji

file modified

+1 -1

		`@@ -444,7 +444,7 @@`
		`symlink_name = "latest-%s-%s" % (compose.conf["release_short"], compose.conf["release_version"])`
		`else:`
		`symlink_name = "latest-%s-%s" % (compose.conf["release_short"], ".".join(compose.conf["release_version"].split(".")[:-1]))`
		`- if compose.conf["release_is_layered"]:`
		`+ if compose.conf.get("base_product_name", ""):`
		`symlink_name += "-%s-%s" % (compose.conf["base_product_short"], compose.conf["base_product_version"])`
		`symlink = os.path.join(compose.topdir, "..", symlink_name)`

doc/configuration.rst

file modified

-5

		`@@ -67,9 +67,6 @@`
		<http://productmd.readthedocs.io/en/latest/common.html#productmd.common.RELEASE_TYPES>`_
		`in productmd documentation.`

		`- release_is_layered = False`
		`- (bool) -- typically False for an operating system, True otherwise`
		`-`
		`release_internal = False`
		`(bool) -- whether the compose is meant for public consumption`

		`@@ -116,8 +113,6 @@`
		`release_short = "rf"`
		`release_version = "23.0"`

		`- release_is_layered = True`
		`-`
		`base_product_name = "Fedora"`
		`base_product_short = "Fedora"`
		`base_product_version = "23"`

pungi/checks.py

file modified

+22 -7

		`@@ -540,7 +540,9 @@`
		`"enum": RELEASE_TYPES,`
		`"default": "ga",`
		`},`
		`- "release_is_layered": {"type": "boolean"},`
		`+ "release_is_layered": {`
		`+ "deprecated": "remove it. It's layered if there's configuration for base product"`
		`+ },`
		`"release_internal": {"type": "boolean", "default": False},`
		`"release_discinfo_description": {"type": "string"},`

		`@@ -1279,7 +1281,6 @@`
		`},`

		`"required": ["release_name", "release_short", "release_version",`
		`- "release_is_layered",`
		`"variants_file",`
		`"runroot", "pkgset_source",`
		`"gather_method"],`
		`@@ -1354,14 +1355,28 @@`
		`(lambda val: not val, ["lorax_options", "buildinstall_kickstart"]),`
		`),`
		`},`
		`- "release_is_layered": {`
		`+ "base_product_name": {`
		`+ "requires": (`
		`+ (lambda x: x, ["base_product_short", "base_product_version"]),`
		`+ ),`
		`+ "conflicts": (`
		`+ (lambda x: not x, ["base_product_short", "base_product_version"]),`
		`+ ),`
		`+ },`
		`+ "base_product_short": {`
		`+ "requires": (`
		`+ (lambda x: x, ["base_product_name", "base_product_version"]),`
		`+ ),`
		`+ "conflicts": (`
		`+ (lambda x: not x, ["base_product_name", "base_product_version"]),`
		`+ ),`
		`+ },`
		`+ "base_product_version": {`
		`"requires": (`
		`- (lambda x: x, ["base_product_name", "base_product_short",`
		`- "base_product_version", "base_product_type"]),`
		`+ (lambda x: x, ["base_product_name", "base_product_short"]),`
		`),`
		`"conflicts": (`
		`- (lambda x: not x, ["base_product_name", "base_product_short",`
		`- "base_product_version", "base_product_type"]),`
		`+ (lambda x: not x, ["base_product_name", "base_product_short"]),`
		`),`
		`},`
		`"runroot": {`

pungi/compose.py

file modified

+1 -1

		`@@ -57,7 +57,7 @@`
		`ci.release.name = conf["release_name"]`
		`ci.release.short = conf["release_short"]`
		`ci.release.version = conf["release_version"]`
		`- ci.release.is_layered = bool(conf.get("release_is_layered", False))`
		`+ ci.release.is_layered = True if conf.get("base_product_name", "") else False`
		`ci.release.type = conf.get("release_type", "ga").lower()`
		`ci.release.internal = bool(conf.get("release_internal", False))`
		`if ci.release.is_layered:`

pungi/metadata.py

file modified

+3 -3

		`@@ -36,7 +36,7 @@`
		`result = "%s %s for %s %s" % (variant.release_name, variant.release_version, compose.conf["release_name"], get_major_version(compose.conf["release_version"]))`
		`else:`
		`result = "%s %s" % (compose.conf["release_name"], compose.conf["release_version"])`
		`- if compose.conf["release_is_layered"]:`
		`+ if compose.conf.get("base_product_name", ""):`
		`result += " for %s %s" % (compose.conf["base_product_name"], compose.conf["base_product_version"])`

		`result = result % {"variant_name": variant.name, "arch": arch}`
		`@@ -78,7 +78,7 @@`
		`ci.release.name = compose.conf["release_name"]`
		`ci.release.version = compose.conf["release_version"]`
		`ci.release.short = compose.conf["release_short"]`
		`- ci.release.is_layered = compose.conf["release_is_layered"]`
		`+ ci.release.is_layered = True if compose.conf.get("base_product_name", "") else False`
		`ci.release.type = compose.conf["release_type"].lower()`
		`ci.release.internal = bool(compose.conf["release_internal"])`

		`@@ -225,7 +225,7 @@`
		`ti.release.name = compose.conf["release_name"]`
		`ti.release.version = compose.conf["release_version"]`
		`ti.release.short = compose.conf["release_short"]`
		`- ti.release.is_layered = compose.conf["release_is_layered"]`
		`+ ti.release.is_layered = True if compose.conf.get("base_product_name", "") else False`
		`ti.release.type = compose.conf["release_type"].lower()`

		`# base product`

pungi/notifier.py

file modified

+3 -2

		`@@ -54,9 +54,10 @@`
		`data.setdefault('release_name', self.compose.conf['release_name'])`
		`data.setdefault('release_version', self.compose.conf['release_version'])`
		`data.setdefault('release_type', self.compose.conf['release_type'].lower())`
		`- data.setdefault('release_is_layered', self.compose.conf["release_is_layered"])`
		`+ data.setdefault('release_is_layered', False)`

		`- if self.compose.conf['release_is_layered']:`
		`+ if self.compose.conf.get('base_product_name', ''):`
		`+ data['release_is_layered'] = True`
		`data['base_product_name'] = self.compose.conf["base_product_name"]`
		`data['base_product_version'] = self.compose.conf["base_product_version"]`
		`data['base_product_short'] = self.compose.conf["base_product_short"]`

pungi/util.py

file modified

+2 -1

		`@@ -397,7 +397,7 @@`
		`else:`
		`release_short = compose.conf["release_short"]`
		`release_version = compose.conf["release_version"]`
		`- release_is_layered = compose.conf["release_is_layered"]`
		`+ release_is_layered = True if compose.conf.get("base_product_name", "") else False`
		`base_product_short = compose.conf.get("base_product_short", "")`
		`base_product_version = compose.conf.get("base_product_version", "")`
		`variant_uid = variant and variant.uid or None`
		`@@ -941,4 +941,5 @@`
		`conf._open_file = file_path`
		`else:`
		`conf.load_from_file(file_path)`
		`+`
		`return conf`

tests/data/dummy-pungi.conf

file modified

-1

		`@@ -2,7 +2,6 @@`
		`release_name = "Dummy Product"`
		`release_short = "DP"`
		`release_version = "1.0"`
		`- release_is_layered = False`
		`release_type = "ga"`

tests/helpers.py

file modified

-1

		`@@ -259,7 +259,6 @@`
		`release_short='test',`
		`release_name='Test',`
		`release_version='1.0',`
		`- release_is_layered=False,`
		`variants_file='variants.xml',`
		`runroot=False,`
		`createrepo_checksum='sha256',`

tests/test_buildinstall.py

file modified

-7

		`@@ -91,7 +91,6 @@`
		`'release_name': 'Test',`
		`'release_short': 't',`
		`'release_version': '1',`
		`- 'release_is_layered': False,`
		`'buildinstall_method': 'lorax',`
		`'disc_types': {'dvd': 'DVD'},`
		`})`
		`@@ -164,7 +163,6 @@`
		`'release_name': 'Test',`
		`'release_short': 't',`
		`'release_version': '1',`
		`- 'release_is_layered': False,`
		`'buildinstall_method': 'lorax'`
		`})`

		`@@ -210,7 +208,6 @@`
		`'release_name': 'Test',`
		`'release_short': 't',`
		`'release_version': '1',`
		`- 'release_is_layered': False,`
		`'buildinstall_method': 'buildinstall',`
		`'disc_types': {'dvd': 'DVD'},`
		`})`
		`@@ -248,7 +245,6 @@`
		`'release_name': 'Test',`
		`'release_short': 't',`
		`'release_version': '1',`
		`- 'release_is_layered': False,`
		`'buildinstall_method': 'lorax',`
		`'lorax_options': [`
		`('^.$', {'': {}}),`
		`@@ -339,7 +335,6 @@`
		`'release_name': 'Test',`
		`'release_short': 't',`
		`'release_version': '1',`
		`- 'release_is_layered': False,`
		`'buildinstall_method': 'lorax',`
		`'lorax_options': [`
		`('^.*$', {`
		`@@ -417,7 +412,6 @@`
		`'release_name': 'Test',`
		`'release_short': 't',`
		`'release_version': '1',`
		`- 'release_is_layered': False,`
		`'buildinstall_method': 'lorax',`
		`'buildinstall_topdir': '/buildinstall_topdir',`
		`'translate_paths': [(self.topdir, "http://localhost/")],`
		`@@ -495,7 +489,6 @@`
		`'release_name': 'Test',`
		`'release_short': 't',`
		`'release_version': '1',`
		`- 'release_is_layered': False,`
		`'buildinstall_method': 'lorax',`
		`'lorax_extra_sources': [`
		`('^Server$', {`

tests/test_compose.py

file modified

-5

		`@@ -253,7 +253,6 @@`
		`release_version='1.0',`
		`release_short='test',`
		`release_type='ga',`
		`- release_is_layered=False,`
		`release_internal=False,`
		`)`

		`@@ -296,7 +295,6 @@`
		`release_version='1.0',`
		`release_short='test',`
		`release_type='ga',`
		`- release_is_layered=False,`
		`release_internal=False,`
		`tree_arches=['x86_64'],`
		`)`
		`@@ -342,7 +340,6 @@`
		`release_version='1.0',`
		`release_short='test',`
		`release_type='ga',`
		`- release_is_layered=False,`
		`release_internal=False,`
		`tree_variants=['Server', 'Client', 'Server-Gluster'],`
		`)`
		`@@ -381,7 +378,6 @@`
		`release_version='1.0',`
		`release_short='test',`
		`release_type='ga',`
		`- release_is_layered=False,`
		`release_internal=False,`
		`tree_variants=['Server', 'Client', 'Server-optional'],`
		`tree_arches=['x86_64'],`
		`@@ -422,7 +418,6 @@`
		`release_version='1.0',`
		`release_short='test',`
		`release_type='ga',`
		`- release_is_layered=False,`
		`release_internal=False,`
		`tree_variants=['Server', 'Client', 'Server-optional'],`
		`tree_arches=['x86_64'],`

tests/test_config.py

file modified

+32 -10

		`@@ -75,7 +75,7 @@`


		`class ReleaseConfigTestCase(ConfigTestCase):`
		`- def test_layered_without_base_product(self):`
		`+ def test_set_release_is_layered(self):`
		`cfg = load_config(`
		`PKGSET_REPOS,`
		`release_is_layered=True`
		`@@ -83,25 +83,47 @@`

		`self.assertValidation(`
		`cfg,`
		`- [checks.REQUIRES.format('release_is_layered', 'True', 'base_product_name'),`
		`- checks.REQUIRES.format('release_is_layered', 'True', 'base_product_short'),`
		`- checks.REQUIRES.format('release_is_layered', 'True', 'base_product_version')])`
		`+ warnings=[`
		`+ "WARNING: Config option release_is_layered was removed and has no effect; remove it. It's layered if there's configuration for base product."])`

		`- def test_not_layered_with_base_product(self):`
		`+ def test_only_config_base_product_name(self):`
		`cfg = load_config(`
		`PKGSET_REPOS,`
		`base_product_name='Prod',`
		`+ )`
		`+`
		`+ self.assertValidation(`
		`+ cfg,`
		`+ [checks.REQUIRES.format('base_product_name', 'Prod', 'base_product_short'),`
		`+ checks.REQUIRES.format('base_product_name', 'Prod', 'base_product_version'),`
		`+ checks.CONFLICTS.format('base_product_short', None, 'base_product_name'),`
		`+ checks.CONFLICTS.format('base_product_version', None, 'base_product_name')])`
		`+`
		`+ def test_only_config_base_product_short(self):`
		`+ cfg = load_config(`
		`+ PKGSET_REPOS,`
		`base_product_short='bp',`
		`+ )`
		`+`
		`+ self.assertValidation(`
		`+ cfg,`
		`+ [checks.REQUIRES.format('base_product_short', 'bp', 'base_product_name'),`
		`+ checks.REQUIRES.format('base_product_short', 'bp', 'base_product_version'),`
		`+ checks.CONFLICTS.format('base_product_name', None, 'base_product_short'),`
		`+ checks.CONFLICTS.format('base_product_version', None, 'base_product_short')])`
		`+`
		`+ def test_only_config_base_product_version(self):`
		`+ cfg = load_config(`
		`+ PKGSET_REPOS,`
		`base_product_version='1.0',`
		`- base_product_type='updates',`
		`)`

		`self.assertValidation(`
		`cfg,`
		`- [checks.CONFLICTS.format('release_is_layered', 'False', 'base_product_name'),`
		`- checks.CONFLICTS.format('release_is_layered', 'False', 'base_product_short'),`
		`- checks.CONFLICTS.format('release_is_layered', 'False', 'base_product_type'),`
		`- checks.CONFLICTS.format('release_is_layered', 'False', 'base_product_version')])`
		`+ [checks.REQUIRES.format('base_product_version', '1.0', 'base_product_name'),`
		`+ checks.REQUIRES.format('base_product_version', '1.0', 'base_product_short'),`
		`+ checks.CONFLICTS.format('base_product_name', None, 'base_product_version'),`
		`+ checks.CONFLICTS.format('base_product_short', None, 'base_product_version')])`


		`class ImageNameConfigTestCase(ConfigTestCase):`

tests/test_createiso_phase.py

file modified

-13

		`@@ -42,7 +42,6 @@`
		`compose = helpers.DummyCompose(self.topdir, {`
		`'release_short': 'test',`
		`'release_version': '1.0',`
		`- 'release_is_layered': False,`
		`'createiso_skip': [`
		`]`
		`})`
		`@@ -75,7 +74,6 @@`
		`compose = helpers.DummyCompose(self.topdir, {`
		`'release_short': 'test',`
		`'release_version': '1.0',`
		`- 'release_is_layered': False,`
		`'createiso_skip': [`
		`]`
		`})`
		`@@ -137,7 +135,6 @@`
		`compose = helpers.DummyCompose(self.topdir, {`
		`'release_short': 'test',`
		`'release_version': '1.0',`
		`- 'release_is_layered': False,`
		`'buildinstall_method': 'lorax',`
		`'bootable': True,`
		`'createiso_skip': []`
		`@@ -221,7 +218,6 @@`
		`compose = helpers.DummyCompose(self.topdir, {`
		`'release_short': 'test',`
		`'release_version': '1.0',`
		`- 'release_is_layered': False,`
		`'buildinstall_method': 'lorax',`
		`'bootable': True,`
		`'createiso_skip': []`
		`@@ -287,7 +283,6 @@`
		`compose = helpers.DummyCompose(self.topdir, {`
		`'release_short': 'test',`
		`'release_version': '1.0',`
		`- 'release_is_layered': False,`
		`'buildinstall_method': 'lorax',`
		`'bootable': True,`
		`'createiso_skip': [],`
		`@@ -353,7 +348,6 @@`
		`compose = helpers.DummyCompose(self.topdir, {`
		`'release_short': 'test',`
		`'release_version': '1.0',`
		`- 'release_is_layered': False,`
		`'runroot': True,`
		`'runroot_tag': 'f25-build',`
		`'koji_profile': 'koji',`
		`@@ -417,7 +411,6 @@`
		`compose = helpers.DummyCompose(self.topdir, {`
		`'release_short': 'test',`
		`'release_version': '1.0',`
		`- 'release_is_layered': False,`
		`'runroot': True,`
		`'runroot_tag': 'f25-build',`
		`'koji_profile': 'koji',`
		`@@ -482,7 +475,6 @@`
		`compose = helpers.DummyCompose(self.topdir, {`
		`'release_short': 'test',`
		`'release_version': '1.0',`
		`- 'release_is_layered': False,`
		`'runroot': True,`
		`'bootable': True,`
		`'buildinstall_method': 'lorax',`
		`@@ -550,7 +542,6 @@`
		`compose = helpers.DummyCompose(self.topdir, {`
		`'release_short': 'test',`
		`'release_version': '1.0',`
		`- 'release_is_layered': False,`
		`'runroot': True,`
		`'runroot_tag': 'f25-build',`
		`'koji_profile': 'koji',`
		`@@ -581,7 +572,6 @@`
		`compose = helpers.DummyCompose(self.topdir, {`
		`'release_short': 'test',`
		`'release_version': '1.0',`
		`- 'release_is_layered': False,`
		`'runroot': True,`
		`'runroot_tag': 'f25-build',`
		`'koji_profile': 'koji',`
		`@@ -620,7 +610,6 @@`
		`compose = helpers.DummyCompose(self.topdir, {`
		`'release_short': 'test',`
		`'release_version': '1.0',`
		`- 'release_is_layered': False,`
		`'runroot': True,`
		`'runroot_tag': 'f25-build',`
		`'koji_profile': 'koji',`
		`@@ -665,7 +654,6 @@`
		`compose = helpers.DummyCompose(self.topdir, {`
		`'release_short': 'test',`
		`'release_version': '1.0',`
		`- 'release_is_layered': False,`
		`'runroot': False,`
		`})`
		`cmd = {`
		`@@ -710,7 +698,6 @@`
		`compose = helpers.DummyCompose(self.topdir, {`
		`'release_short': 'test',`
		`'release_version': '1.0',`
		`- 'release_is_layered': False,`
		`'runroot': False,`
		`'failable_deliverables': [`
		`('^.$', {'': 'iso'})`

tests/test_metadata.py

file modified

-2

		`@@ -25,7 +25,6 @@`
		`compose = helpers.DummyCompose(self.topdir, {`
		`'release_name': 'Test',`
		`'release_version': '1.0',`
		`- 'release_is_layered': False,`
		`})`

		`metadata.write_discinfo(compose, 'x86_64', compose.variants['Server'])`
		`@@ -64,7 +63,6 @@`
		`compose = helpers.DummyCompose(self.topdir, {`
		`'release_name': 'Test',`
		`'release_version': '1.0',`
		`- 'release_is_layered': True,`
		`'base_product_name': 'Base',`
		`'base_product_version': 42,`
		`})`

tests/test_util.py

file modified

-3

		`@@ -245,7 +245,6 @@`
		`conf = {`
		`'release_short': 'rel_short2',`
		`'release_version': '6.0',`
		`- 'release_is_layered': False,`
		`'image_volid_formats': [format],`
		`'image_volid_layered_product_formats': [],`
		`'volume_id_substitutions': {},`
		`@@ -281,7 +280,6 @@`
		`conf = {`
		`'release_short': 'rel_short2',`
		`'release_version': '6.0',`
		`- 'release_is_layered': False,`
		`'image_volid_formats': [format],`
		`'image_volid_layered_product_formats': [],`
		`'volume_id_substitutions': {},`
		`@@ -310,7 +308,6 @@`
		`conf = {`
		`'release_short': 'rel_short2',`
		`'release_version': '6.0',`
		`- 'release_is_layered': False,`
		`'image_volid_formats': [`
		`'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', # 34 chars`
		`'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb', # 33 chars`

hlin commented 4 years ago

Fixes: https://pagure.io/pungi/issue/977
Signed-off-by: Haibo Lin hlin@redhat.com

lsedlar commented 4 years ago

This is looking good so far.

Can you please update documentation in docs/configuration.rst as well?

There are still some references to the option in the code:

pungi_utils/orchestrator.py:505
bin/pungi-koji:447

Minor nitpick: the commit message has a typo: decrecate → deprecate.

rebased onto 1de824d

4 years ago

hlin commented 4 years ago

This is looking good so far.
Can you please update documentation in docs/configuration.rst as well?
There are still some references to the option in the code:

pungi_utils/orchestrator.py:505

This does not reference the option from config file. To my understanding, it's for setting environment var and will be included in the notification msg, is it needed to remove?

bin/pungi-koji:447

Minor nitpick: the commit message has a typo: decrecate → deprecate.

Edited 4 years ago by hlin

lsedlar commented 4 years ago

This does not reference the option from config file. To my understanding, it's for setting environment var and will be included in the notification msg, is it needed to remove?

Oh, right. No need to change anything there.

lsedlar commented 4 years ago

Commit ce06670 fixes this pull-request