PR#1891: Disable notifications from clone-tag by default - koji

koji

#1891 Disable notifications from clone-tag by default

Merged 4 years ago by tkopecek. Opened 4 years ago by cobrien.

cobrien/koji clone-no-notify into master

Disable notifications from clone-tag by default

Christopher O'Brien • 4 years ago

7093cdb

allow python to find its own version of nose rather than looking for a binary which may not exist

Christopher O'Brien • 4 years ago

f0f0e5d

Makefile

file modified

+2 -3

		`@@ -81,7 +81,7 @@`
		`test2:`
		`coverage2 erase`
		`PYTHONPATH=hub/.:plugins/hub/.:plugins/builder/.:plugins/cli/.:cli/.:www/lib coverage2 run \`
		`- --source . /usr/bin/nosetests`
		`+ --source . -m nose`
julian8628 commented 4 years ago is the previous cmd broken on some OS?
		`coverage2 report`
		`coverage2 html`
		`@echo Full coverage report at file://${CURDIR}/htmlcov/py2/index.html`
		`@@ -89,8 +89,7 @@`
		`test3:`
		`coverage3 erase --rcfile .coveragerc3`
		`PYTHONPATH=hub/.:plugins/hub/.:plugins/builder/.:plugins/cli/.:cli/.:www/lib coverage3 run \`
		`- --rcfile .coveragerc3 --source . \`
		`- /usr/bin/nosetests-3`
		`+ --rcfile .coveragerc3 --source . -m nose`
		`coverage3 report --rcfile .coveragerc3`
		`coverage3 html --rcfile .coveragerc3`
		`@echo Full coverage report at file://${CURDIR}/htmlcov/py3/index.html`

cli/koji_cli/commands.py

file modified

+10 -4

		`@@ -3249,6 +3249,8 @@`
		`help=_('Clone tag at a specific repo event'))`
		`parser.add_option("-v", "--verbose", action="store_true",`
		`help=_("show changes"))`
		`+ parser.add_option("--notify", action="store_true", default=False,`
		`+ help=_('Send tagging/untagging notifications'))`
		`parser.add_option("-f", "--force", action="store_true",`
		`help=_("override tag locks if necessary"))`
		`parser.add_option("-n", "--test", action="store_true", help=_("test mode"))`
		`@@ -3348,7 +3350,8 @@`
		`if not options.test:`
		`session.tagBuildBypass(newtag['name'],`
		`build,`
		`- force=options.force)`
		`+ force=options.force,`
		`+ notify=options.notify)`
		`if not options.test:`
		`session.multiCall(batch=options.batch)`
		`if options.groups:`
		`@@ -3523,7 +3526,8 @@`
		`if not options.test:`
		`session.untagBuildBypass(dsttag['name'],`
		`build,`
		`- force=options.force)`
		`+ force=options.force,`
		`+ notify=options.notify)`
		`if not options.test:`
		`session.multiCall(batch=options.batch)`
		`# ADD builds.`
		`@@ -3542,7 +3546,8 @@`
		`if not options.test:`
		`session.tagBuildBypass(dsttag['name'],`
		`build,`
		`- force=options.force)`
		`+ force=options.force,`
		`+ notify=options.notify)`
		`if not options.test:`
		`session.multiCall(batch=options.batch)`
		`# ADD groups.`
		`@@ -3608,7 +3613,8 @@`
		`if not options.test:`
		`session.untagBuildBypass(dsttag['name'],`
		`build,`
		`- force=options.force)`
		`+ force=options.force,`
		`+ notify=options.notify)`
		`# now safe to remove package itself since we resolved its builds.`
		`chgpkglist.append(('[del]',`
		`pkg['package_name'],`

tests/test_cli/test_clone_tag.py

file modified

+21 -10

		`@@ -244,25 +244,29 @@`
		`'nvr': 'pkg2-1.0-1',`
		`'package_name': 'pkg2', 'state': 2,`
		`'tag_name': 'src-tag-p',`
		`- 'name': 'pkg2'}, force=None),`
		`+ 'name': 'pkg2'}, force=None,`
		`+ notify=False),`
julian8628 commented 4 years ago the indents here and below are not consistent
		`call.tagBuildBypass('dst-tag', {`
		`'owner_name': 'b_owner',`
		`'nvr': 'pkg1-1.0-1',`
		`'package_name': 'pkg1', 'state': 1,`
		`'tag_name': 'src-tag',`
		`- 'name': 'pkg1'}, force=None),`
		`+ 'name': 'pkg1'}, force=None,`
		`+ notify=False),`
		`call.tagBuildBypass('dst-tag', {`
		`'owner_name': 'b_owner',`
		`'nvr': 'pkg1-1.0-2',`
		`'package_name': 'pkg1', 'state': 1,`
		`'tag_name': 'src-tag',`
		`- 'name': 'pkg1'}, force=None),`
		`+ 'name': 'pkg1'}, force=None,`
		`+ notify=False),`
		`call.tagBuildBypass('dst-tag', {`
		`'owner_name': 'b_owner',`
		`'nvr': 'pkg1-1.1-2',`
		`'package_name': 'pkg1', 'state': 1,`
		`'tag_name': 'src-tag',`
		`- 'name': 'pkg1'}, force=None),`
		`+ 'name': 'pkg1'}, force=None,`
		`+ notify=False),`
		`call.multiCall(batch=1000),`
		`call.getTagGroups('src-tag',`
		`event=None),`
		`@@ -506,38 +510,44 @@`
		`'nvr': 'pkg1-2.1-2',`
		`'package_name': 'pkg1', 'state': 1,`
		`'tag_name': 'dst-tag',`
		`- 'name': 'pkg1'}, force=None),`
		`+ 'name': 'pkg1'}, force=None,`
		`+ notify=False),`
		`call.untagBuildBypass('dst-tag', {`
		`'owner_name': 'b_owner',`
		`'nvr': 'pkg1-0.1-1',`
		`'package_name': 'pkg1', 'state': 1,`
		`'tag_name': 'dst-tag',`
		`- 'name': 'pkg1'}, force=None),`
		`+ 'name': 'pkg1'}, force=None,`
		`+ notify=False),`
		`call.untagBuildBypass('dst-tag', {`
		`'owner_name': 'b_owner',`
		`'nvr': 'pkg3-1.0-1',`
		`'package_name': 'pkg3', 'state': 1,`
		`'tag_name': 'dst-tag',`
		`- 'name': 'pkg3'}, force=None),`
		`+ 'name': 'pkg3'}, force=None,`
		`+ notify=False),`
		`call.multiCall(batch=1000),`
		`call.tagBuildBypass('dst-tag', {`
		`'owner_name': 'b_owner',`
		`'nvr': 'pkg1-0.1-1',`
		`'package_name': 'pkg1', 'state': 1,`
		`'tag_name': 'src-tag',`
		`- 'name': 'pkg1'}, force=None),`
		`+ 'name': 'pkg1'}, force=None,`
		`+ notify=False),`
		`call.tagBuildBypass('dst-tag', {`
		`'owner_name': 'b_owner',`
		`'nvr': 'pkg1-1.0-2',`
		`'package_name': 'pkg1', 'state': 1,`
		`'tag_name': 'src-tag',`
		`- 'name': 'pkg1'}, force=None),`
		`+ 'name': 'pkg1'}, force=None,`
		`+ notify=False),`
		`call.tagBuildBypass('dst-tag', {`
		`'owner_name': 'b_owner',`
		`'nvr': 'pkg1-1.1-2',`
		`'package_name': 'pkg1', 'state': 1,`
		`'tag_name': 'src-tag',`
		`- 'name': 'pkg1'}, force=None),`
		`+ 'name': 'pkg1'}, force=None,`
		`+ notify=False),`
		`call.multiCall(batch=1000),`
		`call.multiCall(batch=1000),`
		`call.groupPackageListAdd('dst-tag',`
		`@@ -633,6 +643,7 @@`
		`--event=EVENT Clone tag at a specific event`
		`--repo=REPO Clone tag at a specific repo event`
		`-v, --verbose show changes`
		`+ --notify Send tagging/untagging notifications`
		`-f, --force override tag locks if necessary`
		`-n, --test test mode`
		`--batch=SIZE batch size of multicalls [0 to disable, default: 1000]`

cobrien commented 4 years ago

The tagBuildBypass and untagBuildBypass invocations in the clone-tag CLI were not setting the notify option, leaving it at the default of True. This means in some cases many hundreds of notification events would be triggered from a clone-tag call.

Change the default behavior of clone-tag to explicitly disable notifications
Add a new --notify option to clone-tag to re-enable notifications for tagging and untagging of builds.

Fixes: #1890

cobrien commented 4 years ago

cc @mikem @tkopecek @breilly

julian8628 commented on line 6 of Makefile 4 years ago

is the previous cmd broken on some OS?

julian8628 commented on line 7 of tests/test_cli/test_clone_tag.py 4 years ago

the indents here and below are not consistent

cobrien commented 4 years ago

the indents here and below are not consistent

The crazy thing there is that the indents actually are consistent, because that's the first indented argument! The previous indentations are a dict that is split over multiple lines. If you'd like I can indent the force parameter as well -- it would then line up with the first argument and the notify argument.

cobrien commented 4 years ago

is the previous cmd broken on some OS?

Yes, in many cases there is no /usr/bin/nosetests (but instead a nosetests-2). This also fails situations that work with a virtualenv or alternative paths. But asking coverage to invoke the nose module that it finds on its own always works

Edited 4 years ago by cobrien

julian8628 commented 4 years ago

the indents here and below are not consistent

The crazy thing there is that the indents actually are consistent, because that's the first indented argument! The previous indentations are a dict that is split over multiple lines. If you'd like I can indent the force parameter as well -- it would then line up with the first argument and the notify argument.

ah, ok. then :thumbsup: