PR#778: add history to edit_host - koji

		`@@ -2772,9 +2772,9 @@`
		`host['arches'] = ','.join(host['arches'].split())`

		`if not options.quiet:`
		`- print("Hostname Enb Rdy Load/Cap Arches Last Update")`
		`+ print("Hostname Enb Rdy Load/Cap Arches Last Update")`
		`for host in hosts:`
		`- print("%(name)-28s %(enabled)-3s %(ready)-3s %(task_load)4.1f/%(capacity)-3.1f %(arches)-16s %(update)s" % host)`
		`+ print("%(name)-28s %(enabled)-3s %(ready)-3s %(task_load)4.1f/%(capacity)-4.1f %(arches)-16s %(update)s" % host)`


		`def anon_handle_list_pkgs(goptions, session, args):`
		`@@ -4027,6 +4027,18 @@`
		`fmt = "added tag option %(key)s for tag %(tag.name)s"`
		`else:`
		`fmt = "tag option %(key)s removed for %(tag.name)s"`
		`+ elif table == 'host_config':`
		`+ if edit:`
		`+ fmt = "host configuration for %(host.name)s altered"`
		`+ elif create:`
		`+ fmt = "new host: %(host.name)s"`
		`+ else:`
		`+ fmt = "host deleted: %(host.name)s"`
		`+ elif table == 'host_channels':`
		`+ if create:`
		`+ fmt = "host %(host.name)s added to channel %(channels.name)s"`
		`+ else:`
		`+ fmt = "host %(host.name)s removed from channel %(channels.name)s"`
		`elif table == 'build_target_config':`
		`if edit:`
		`fmt = "build target configuration for %(build_target.name)s updated"`
		`@@ -4141,6 +4153,8 @@`
		`'tag_extra' : ['tag_id', 'key'],`
		`'build_target_config' : ['build_target_id'],`
		`'external_repo_config' : ['external_repo_id'],`
		`+ 'host_config': ['host_id'],`
		`+ 'host_channels': ['host_id'],`
		`'tag_external_repos' : ['tag_id', 'external_repo_id'],`
		`'tag_listing' : ['build_id', 'tag_id'],`
		`'tag_packages' : ['package_id', 'tag_id'],`

docs/schema-upgrade-1.15-1.16.sql

file added

+64

		`@@ -0,0 +1,64 @@`
		`+ -- upgrade script to migrate the Koji database schema`
		`+ -- from version 1.14 to 1.16`
		`+`
		`+`
		`+ BEGIN;`
		`+`
		`+ -- create host_config table`
		`+ SELECT 'Creating table host_config';`
		`+ CREATE TABLE host_config (`
		`+ host_id INTEGER NOT NULL REFERENCES host(id),`
		`+ arches TEXT,`
		`+ capacity FLOAT CHECK (capacity > 1) NOT NULL DEFAULT 2.0,`
		`+ description TEXT,`
		`+ comment TEXT,`
		`+ enabled BOOLEAN NOT NULL DEFAULT 'true',`
		`+ -- versioned - see desc above`
		`+ create_event INTEGER NOT NULL REFERENCES events(id) DEFAULT get_event(),`
		`+ revoke_event INTEGER REFERENCES events(id),`
		`+ creator_id INTEGER NOT NULL REFERENCES users(id),`
		`+ revoker_id INTEGER REFERENCES users(id),`
		`+ active BOOLEAN DEFAULT 'true' CHECK (active),`
		`+ CONSTRAINT active_revoke_sane CHECK (`
		`+ (active IS NULL AND revoke_event IS NOT NULL AND revoker_id IS NOT NULL)`
		`+ OR (active IS NOT NULL AND revoke_event IS NULL AND revoker_id IS NULL)),`
		`+ PRIMARY KEY (create_event, host_id),`
		`+ UNIQUE (host_id, active)`
		`+ ) WITHOUT OIDS;`
		`+ CREATE INDEX host_config_by_active_and_enabled ON host_config(active, enabled)`
		`+`
		`+ -- copy starting data`
		`+ -- CREATE FUNCTION pg_temp.user() returns INTEGER as $$ select id from users where name='nobody' $$ language SQL;`
		`+ CREATE FUNCTION pg_temp.user() returns INTEGER as $$ select 1 $$ language SQL;`
		`+ -- If you would like to use an existing user instead, then:`
		`+ -- 1. edit the temporary function to look for the alternate user name`
		`+`
		`+ SELECT 'Copying data from host to host_config';`
		`+ INSERT INTO host_config (host_id, arches, capacity, description, comment, enabled, creator_id)`
		`+ SELECT id, arches, capacity, description, comment, enabled, pg_temp.user() FROM host;`
		`+`
		`+ -- alter original table`
		`+ SELECT 'Dropping moved columns';`
		`+ ALTER TABLE host DROP COLUMN arches;`
		`+ ALTER TABLE host DROP COLUMN capacity;`
		`+ ALTER TABLE host DROP COLUMN description;`
		`+ ALTER TABLE host DROP COLUMN comment;`
		`+ ALTER TABLE host DROP COLUMN enabled;`
		`+`
		`+ -- history for host_channels`
		`+ SELECT 'Adding versions to host_channels'`
		`+ ALTER TABLE host_channels ADD COLUMN create_event INTEGER NOT NULL REFERENCES events(id) DEFAULT get_event();`
		`+ ALTER TABLE host_channels ADD COLUMN revoke_event INTEGER REFERENCES events(id);`
		`+ -- we need some default for alter table, but drop it after`
		`+ ALTER TABLE host_channels ADD COLUMN creator_id INTEGER NOT NULL REFERENCES users(id) DEFAULT pg_temp.user();`
		`+ ALTER TABLE host_channels ALTER COLUMN creator_id DROP DEFAULT;`
		`+ ALTER TABLE host_channels ADD COLUMN revoker_id INTEGER REFERENCES users(id);`
		`+ ALTER TABLE host_channels ADD COLUMN active BOOLEAN DEFAULT 'true' CHECK (active);`
		`+ ALTER TABLE host_channels ADD CONSTRAINT active_revoke_sane CHECK (`
		`+ (active IS NULL AND revoke_event IS NOT NULL AND revoker_id IS NOT NULL)`
		`+ OR (active IS NOT NULL AND revoke_event IS NULL AND revoker_id IS NULL));`
		`+ ALTER TABLE host_channels ADD PRIMARY KEY (create_event, host_id, channel_id);`
		`+ ALTER TABLE host_channels ADD UNIQUE (host_id, channel_id, active);`
		`+ ALTER TABLE host_channels DROP CONSTRAINT host_channels_host_id_channel_id_key;`
		`+`
		`+ COMMIT;`

docs/schema.sql

file modified

+30 -5

		`@@ -145,20 +145,45 @@`
		`id SERIAL NOT NULL PRIMARY KEY,`
		`user_id INTEGER NOT NULL REFERENCES users (id),`
		`name VARCHAR(128) UNIQUE NOT NULL,`
		`- arches TEXT,`
		`task_load FLOAT CHECK (NOT task_load < 0) NOT NULL DEFAULT 0.0,`
		`+ ready BOOLEAN NOT NULL DEFAULT 'false',`
		`+ ) WITHOUT OIDS;`
		`+`
		`+ CREATE TABLE host_config (`
		`+ host_id INTEGER NOT NULL REFERENCES host(id),`
		`+ arches TEXT,`
		`capacity FLOAT CHECK (capacity > 1) NOT NULL DEFAULT 2.0,`
		`description TEXT,`
		`comment TEXT,`
		`- ready BOOLEAN NOT NULL DEFAULT 'false',`
		`- enabled BOOLEAN NOT NULL DEFAULT 'true'`
		`+ enabled BOOLEAN NOT NULL DEFAULT 'true',`
		`+ -- versioned - see desc above`
		`+ create_event INTEGER NOT NULL REFERENCES events(id) DEFAULT get_event(),`
		`+ revoke_event INTEGER REFERENCES events(id),`
		`+ creator_id INTEGER NOT NULL REFERENCES users(id),`
		`+ revoker_id INTEGER REFERENCES users(id),`
		`+ active BOOLEAN DEFAULT 'true' CHECK (active),`
		`+ CONSTRAINT active_revoke_sane CHECK (`
		`+ (active IS NULL AND revoke_event IS NOT NULL AND revoker_id IS NOT NULL)`
		`+ OR (active IS NOT NULL AND revoke_event IS NULL AND revoker_id IS NULL)),`
		`+ PRIMARY KEY (create_event, host_id),`
		`+ UNIQUE (host_id, active)`
		`) WITHOUT OIDS;`
		`- CREATE INDEX HOST_IS_READY_AND_ENABLED ON host(enabled, ready) WHERE (enabled IS TRUE AND ready IS TRUE);`
		`+ CREATE INDEX host_config_by_active_and_enabled ON host_config(active, enabled)`

		`CREATE TABLE host_channels (`
		`host_id INTEGER NOT NULL REFERENCES host(id),`
		`channel_id INTEGER NOT NULL REFERENCES channels(id),`
		`- UNIQUE (host_id,channel_id)`
		`+ -- versioned - see desc above`
		`+ create_event INTEGER NOT NULL REFERENCES events(id) DEFAULT get_event(),`
		`+ revoke_event INTEGER REFERENCES events(id),`
		`+ creator_id INTEGER NOT NULL REFERENCES users(id),`
		`+ revoker_id INTEGER REFERENCES users(id),`
		`+ active BOOLEAN DEFAULT 'true' CHECK (active),`
		`+ CONSTRAINT active_revoke_sane CHECK (`
		`+ (active IS NULL AND revoke_event IS NOT NULL AND revoker_id IS NOT NULL)`
		`+ OR (active IS NOT NULL AND revoke_event IS NULL AND revoker_id IS NULL)),`
		`+ PRIMARY KEY (create_event, host_id, channel_id),`
		`+ UNIQUE (host_id, channel_id, active)`
		`) WITHOUT OIDS;`

hub/kojihub.py

file modified

+132 -58

		`@@ -2032,11 +2032,18 @@`

		`def set_host_enabled(hostname, enabled=True):`
		`context.session.assertPerm('admin')`
		`- if not get_host(hostname):`
		`+ host = get_host(hostname)`
		`+ if not host:`
		`raise koji.GenericError('host does not exist: %s' % hostname)`
		`- c = context.cnx.cursor()`
		`- c.execute("""UPDATE host SET enabled = %(enabled)s WHERE name = %(hostname)s""", locals())`
		`- context.commit_pending = True`
		`+`
		`+ update = UpdateProcessor('host_config', values=host, clauses=['host_id = %(id)i'])`
		`+ update.make_revoke()`
		`+ update.execute()`
		`+`
		`+ insert = InsertProcessor('host_config', data=dslice(host, ('user_id', 'name', 'arches', 'capacity', 'description', 'comment', 'enabled')))`
		`+ insert.set(host_id=host['id'], enabled=enabled)`
		`+ insert.make_create()`
		`+ insert.execute()`

		`def add_host_to_channel(hostname, channel_name, create=False):`
		`"""Add the host to the specified channel`
		`@@ -2057,6 +2064,7 @@`
		`raise koji.GenericError('host %s is already subscribed to the %s channel' % (hostname, channel_name))`
		`insert = InsertProcessor('host_channels')`
		`insert.set(host_id=host_id, channel_id=channel_id)`
		`+ insert.make_create()`
		`insert.execute()`

		`def remove_host_from_channel(hostname, channel_name):`
		`@@ -2076,9 +2084,13 @@`
		`break`
		`if not found:`
		`raise koji.GenericError('host %s is not subscribed to the %s channel' % (hostname, channel_name))`
		`- c = context.cnx.cursor()`
		`- c.execute("""DELETE FROM host_channels WHERE host_id = %(host_id)d and channel_id = %(channel_id)d""", locals())`
		`- context.commit_pending = True`
		`+`
		`+ values = {'host_id': host_id, 'channel_id': channel_id}`
		`+ clauses = ['host_id = %(host_id)i AND channel_id = %(channel_id)i']`
		`+ update = UpdateProcessor('host_channels', values=values, clauses=clauses)`
		`+ update.make_revoke()`
		`+ update.execute()`
		`+`

		`def rename_channel(old, new):`
		`"""Rename a channel"""`
		`@@ -2098,6 +2110,10 @@`

		`Channel must have no hosts, unless force is set to True`
		`If a channel has associated tasks, it cannot be removed`
		`+ and an exception will be raised.`
		`+`
		`+ Removing channel will remove also remove complete history`
		`+ for that channel.`
		`"""`
		`context.session.assertPerm('admin')`
		`channel_id = get_channel_id(channel_name, strict=True)`
		`@@ -2130,16 +2146,18 @@`
		`q = """`
		`SELECT %s FROM host`
		`JOIN sessions USING (user_id)`
		`+ JOIN host_config ON host.id = host_config.host_id`
		`WHERE enabled = TRUE AND ready = TRUE`
		`AND expired = FALSE`
		`AND master IS NULL`
		`AND update_time > NOW() - '5 minutes'::interval`
		`+ AND active IS TRUE`
		`""" % ','.join(fields)`
		`# XXX - magic number in query`
		`c.execute(q)`
		`hosts = [dict(zip(aliases, row)) for row in c.fetchall()]`
		`for host in hosts:`
		`- q = """SELECT channel_id FROM host_channels WHERE host_id=%(id)s"""`
		`+ q = """SELECT channel_id FROM host_channels WHERE host_id=%(id)s AND active IS TRUE"""`
		`c.execute(q, host)`
		`host['channels'] = [row[0] for row in c.fetchall()]`
		`return hosts`
		`@@ -2147,7 +2165,7 @@`
		`def get_all_arches():`
		`"""Return a list of all (canonical) arches available from hosts"""`
		`ret = {}`
		`- for (arches,) in _fetchMulti('SELECT arches FROM host', {}):`
		`+ for (arches,) in _fetchMulti('SELECT arches FROM host_config WHERE active IS TRUE', {}):`
		`if arches is None:`
		`continue`
		`for arch in arches.split():`
		`@@ -4483,7 +4501,7 @@`
		`context.commit_pending = True`
		`return ret`

		`- def get_host(hostInfo, strict=False):`
		`+ def get_host(hostInfo, strict=False, event=None):`
		`"""Get information about the given host. hostInfo may be`
		`either a string (hostname) or int (host id). A map will be returned`
		`containing the following data:`
		`@@ -4499,18 +4517,39 @@`
		`- ready`
		`- enabled`
		`"""`
		`- fields = ('id', 'user_id', 'name', 'arches', 'task_load',`
		`- 'capacity', 'description', 'comment', 'ready', 'enabled')`
		`- query = """SELECT %s FROM host`
		`- WHERE """ % ', '.join(fields)`
		`- if isinstance(hostInfo, int) or isinstance(hostInfo, long):`
		`- query += """id = %(hostInfo)i"""`
		`+ tables = ['host_config']`
		`+ joins = ['host ON host.id = host_config.host_id']`
		`+`
		`+ fields = {'host.id': 'id',`
		`+ 'host.user_id': 'user_id',`
		`+ 'host.name': 'name',`
		`+ 'host.ready': 'ready',`
		`+ 'host.task_load': 'task_load',`
		`+ 'host_config.arches': 'arches',`
		`+ 'host_config.capacity': 'capacity',`
		`+ 'host_config.description': 'description',`
		`+ 'host_config.comment': 'comment',`
		`+ 'host_config.enabled': 'enabled',`
		`+ }`
		`+ clauses = [eventCondition(event, table='host_config')]`
		`+`
		`+ if isinstance(hostInfo, (int, long)):`
		`+ clauses.append("host.id = %(hostInfo)i")`
		`elif isinstance(hostInfo, str):`
		`- query += """name = %(hostInfo)s"""`
		`+ clauses.append("host.name = %(hostInfo)s")`
		`else:`
		`raise koji.GenericError('invalid type for hostInfo: %s' % type(hostInfo))`

		`- return _singleRow(query, locals(), fields, strict)`
		`+ data = {'hostInfo': hostInfo}`
		`+ fields, aliases = zip(*fields.items())`
		`+ query = QueryProcessor(columns=fields, aliases=aliases, tables=tables,`
		`+ joins=joins, clauses=clauses, values=data)`
		`+ result = query.executeOne()`
		`+ if not result:`
		`+ if strict:`
		`+ raise koji.GenericError('Invalid hostInfo: %s' % hostInfo)`
		`+ return None`
		`+ return result`

		`def edit_host(hostInfo, **kw):`
		`"""Edit information for an existing host.`
		`@@ -4532,19 +4571,22 @@`
		`changes = []`
		`for field in fields:`
		`if field in kw and kw[field] != host[field]:`
		`- if field == 'capacity':`
		`- # capacity is a float, so set the substitution format appropriately`
		`- changes.append('%s = %%(%s)f' % (field, field))`
		`- else:`
		`- changes.append('%s = %%(%s)s' % (field, field))`
		`+ changes.append(field)`

		`if not changes:`
		`return False`

		`- update = 'UPDATE host set ' + ', '.join(changes) + ' where id = %(id)i'`
		`- data = kw.copy()`
		`- data['id'] = host['id']`
		`- _dml(update, data)`
		`+ update = UpdateProcessor('host_config', values=host, clauses=['host_id = %(id)i'])`
		`+ update.make_revoke()`
		`+ update.execute()`
		`+`
		`+ insert = InsertProcessor('host_config', data=dslice(host, ('arches', 'capacity', 'description', 'comment', 'enabled')))`
		`+ insert.set(host_id=host['id'])`
		`+ for change in changes:`
		`+ insert.set(**{change: kw[change]})`
		`+ insert.make_create()`
		`+ insert.execute()`
		`+`
		`return True`

		`def get_channel(channelInfo, strict=False):`
		`@@ -4652,16 +4694,28 @@`
		`raise koji.GenericError("More that one buildroot with id: %i" % buildrootID)`
		`return result[0]`

		`- def list_channels(hostID=None):`
		`+ def list_channels(hostID=None, event=None):`
		`"""List channels. If hostID is specified, only list`
		`channels associated with the host with that ID."""`
		`- fields = ('id', 'name')`
		`- query = """SELECT %s FROM channels`
		`- """ % ', '.join(fields)`
		`- if hostID != None:`
		`- query += """JOIN host_channels ON channels.id = host_channels.channel_id`
		`- WHERE host_channels.host_id = %(hostID)i"""`
		`- return _multiRow(query, locals(), fields)`
		`+ fields = {'channels.id': 'id', 'channels.name': 'name'}`
		`+ columns, aliases = zip(*fields.items())`
		`+ if hostID:`
		`+ tables = ['host_channels']`
		`+ joins = ['channels ON channels.id = host_channels.channel_id']`
		`+ clauses = [`
		`+ eventCondition(event, table='host_channels'),`
		`+ 'host_channels.host_id = %(host_id)s']`
		`+ values = {'host_id': hostID}`
		`+ query = QueryProcessor(tables=tables, aliases=aliases,`
		`+ columns=columns, joins=joins,`
		`+ clauses=clauses, values=values)`
		`+ elif event:`
		`+ raise koji.GenericError('list_channels with event and '`
		`+ 'not host is not allowed.')`
		`+ else:`
		`+ query = QueryProcessor(tables=['channels'], aliases=aliases,`
		`+ columns=columns)`
		`+ return query.execute()`

		`def new_package(name, strict=True):`
		`c = context.cnx.cursor()`
		`@@ -6521,6 +6575,8 @@`
		`'tag_extra': ['tag_id', 'key', 'value'],`
		`'build_target_config': ['build_target_id', 'build_tag', 'dest_tag'],`
		`'external_repo_config': ['external_repo_id', 'url'],`
		`+ 'host_config': ['host_id', 'arches', 'capacity', 'description', 'comment', 'enabled'],`
		`+ 'host_channels': ['host_id', 'channel_id'],`
		`'tag_external_repos': ['tag_id', 'external_repo_id', 'priority'],`
		`'tag_listing': ['build_id', 'tag_id'],`
		`'tag_packages': ['package_id', 'tag_id', 'owner', 'blocked', 'extra_arches'],`
		`@@ -6537,6 +6593,8 @@`
		`'cg_id': ['content_generator'],`
		`#group_id is overloaded (special case below)`
		`'tag_id': ['tag'],`
		`+ 'host_id': ['host'],`
		`+ 'channel_id': ['channels'],`
		`'parent_id': ['tag', 'parent'],`
		`'build_target_id': ['build_target'],`
		`'build_tag': ['tag', 'build_tag'],`
		`@@ -10542,14 +10600,20 @@`
		`krb_principal=krb_principal)`
		`#host entry`
		`hostID = _singleValue("SELECT nextval('host_id_seq')", strict=True)`
		`- arches = " ".join(arches)`
		`- insert = """INSERT INTO host (id, user_id, name, arches)`
		`- VALUES (%(hostID)i, %(userID)i, %(hostname)s, %(arches)s)"""`
		`- _dml(insert, locals())`
		`+ insert = "INSERT INTO host (id, user_id, name) VALUES (%(hostID)i, %(userID)i, %(hostname)s)"`
		`+ _dml(insert, dslice(locals(), ('hostID', 'userID', 'hostname')))`
		`+`
		`+ insert = InsertProcessor('host_config')`
		`+ insert.set(host_id=hostID, arches=" ".join(arches))`
		`+ insert.make_create()`
		`+ insert.execute()`
		`+`
		`#host_channels entry`
		`- insert = """INSERT INTO host_channels (host_id, channel_id)`
		`- VALUES (%(hostID)i, %(default_channel)i)"""`
		`- _dml(insert, locals())`
		`+ insert = InsertProcessor('host_channels')`
		`+ insert.set(host_id=hostID, channel_id=default_channel)`
		`+ insert.make_create()`
		`+ insert.execute()`
		`+`
		`return hostID`

		`def enableHost(self, hostname):`
		`@@ -10573,11 +10637,8 @@`
		`host appears in the list, it will be included in the results. If "ready" and "enabled"`
		`are specified, only hosts with the given value for the respective field will`
		`be included."""`
		`- fields = ('id', 'user_id', 'name', 'arches', 'task_load',`
		`- 'capacity', 'description', 'comment', 'ready', 'enabled')`
		`-`
		`- clauses = []`
		`- joins = []`
		`+ clauses = ['host_config.active IS TRUE']`
		`+ joins = ['host ON host.id = host_config.host_id']`
		`if arches is not None:`
		`if not arches:`
		`raise koji.GenericError('arches option cannot be empty')`
		`@@ -10589,25 +10650,38 @@`
		`clauses.append('(' + ' OR '.join(archClause) + ')')`
		`if channelID is not None:`
		`channelID = get_channel_id(channelID, strict=True)`
		`- joins.append('host_channels on host.id = host_channels.host_id')`
		`+ joins.append('host_channels ON host.id = host_channels.host_id')`
		`clauses.append('host_channels.channel_id = %(channelID)i')`
		`+ clauses.append('host_channels.active IS TRUE')`
		`if ready is not None:`
		`if ready:`
		`- clauses.append('ready is true')`
		`+ clauses.append('ready IS TRUE')`
		`else:`
		`- clauses.append('ready is false')`
		`+ clauses.append('ready IS FALSE')`
		`if enabled is not None:`
		`if enabled:`
		`- clauses.append('enabled is true')`
		`+ clauses.append('enabled IS TRUE')`
		`else:`
		`- clauses.append('enabled is false')`
		`+ clauses.append('enabled IS FALSE')`
		`if userID is not None:`
		`userID = get_user(userID, strict=True)['id']`
		`clauses.append('user_id = %(userID)i')`

		`- query = QueryProcessor(columns=fields, tables=['host'],`
		`- joins=joins, clauses=clauses,`
		`- values=locals(), opts=queryOpts)`
		`+ fields = {'host.id': 'id',`
		`+ 'host.user_id': 'user_id',`
		`+ 'host.name': 'name',`
		`+ 'host.ready': 'ready',`
		`+ 'host.task_load': 'task_load',`
		`+ 'host_config.arches': 'arches',`
		`+ 'host_config.capacity': 'capacity',`
		`+ 'host_config.description': 'description',`
		`+ 'host_config.comment': 'comment',`
		`+ 'host_config.enabled': 'enabled',`
		`+ }`
		`+ tables = ['host_config']`
		`+ fields, aliases = zip(*fields.items())`
		`+ query = QueryProcessor(columns=fields, aliases=aliases,`
		`+ tables=tables, joins=joins, clauses=clauses, values=locals())`
		`return query.execute()`

		`def getLastHostUpdate(self, hostID):`
		`@@ -11464,12 +11538,12 @@`
		`id = self.id`
		`#get arch and channel info for host`
		`q = """`
		`- SELECT arches FROM host WHERE id = %(id)s`
		`+ SELECT arches FROM host_config WHERE id = %(id)s AND active IS TRUE`
		`"""`
		`c.execute(q, locals())`
		`arches = c.fetchone()[0].split()`
		`q = """`
		`- SELECT channel_id FROM host_channels WHERE host_id = %(id)s`
		`+ SELECT channel_id FROM host_channels WHERE host_id = %(id)s AND active is TRUE`
		`"""`
		`c.execute(q, locals())`
		`channels = [x[0] for x in c.fetchall()]`
		`@@ -11506,7 +11580,7 @@`

		`def isEnabled(self):`
		`"""Return whether this host is enabled or not."""`
		`- query = """SELECT enabled FROM host WHERE id = %(id)i"""`
		`+ query = """SELECT enabled FROM host_config WHERE id = %(id)i AND active IS TRUE"""`
		`return _singleValue(query, {'id': self.id}, strict=True)`

		`class HostExports(object):`

tests/test_hub/test_add_host.py

file added

+75

		`@@ -0,0 +1,75 @@`
		`+ import unittest`
		`+ import mock`
		`+`
		`+ import koji`
		`+ import kojihub`
		`+`
		`+ UP = kojihub.UpdateProcessor`
		`+ IP = kojihub.InsertProcessor`
		`+`
		`+`
		`+ class TestAddHost(unittest.TestCase):`
		`+ def getInsert(self, args, *kwargs):`
		`+ insert = IP(args, *kwargs)`
		`+ insert.execute = mock.MagicMock()`
		`+ self.inserts.append(insert)`
		`+ return insert`
		`+`
		`+ def getUpdate(self, args, *kwargs):`
		`+ update = UP(args, *kwargs)`
		`+ update.execute = mock.MagicMock()`
		`+ self.updates.append(update)`
		`+ return update`
		`+`
		`+ def setUp(self):`
		`+ self.InsertProcessor = mock.patch('kojihub.InsertProcessor',`
		`+ side_effect=self.getInsert).start()`
		`+ self.inserts = []`
		`+ self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor',`
		`+ side_effect=self.getUpdate).start()`
		`+ self.updates = []`
		`+ self.context = mock.patch('kojihub.context').start()`
		`+ # It seems MagicMock will not automatically handle attributes that`
		`+ # start with "assert"`
		`+ self.context.session.assertLogin = mock.MagicMock()`
		`+ self.context.session.assertPerm = mock.MagicMock()`
		`+ self.context.opts = {'HostPrincipalFormat': '-%s-'}`
		`+ self.exports = kojihub.RootExports()`
		`+`
		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`+ @mock.patch('kojihub._dml')`
		`+ @mock.patch('kojihub.get_host')`
		`+ @mock.patch('kojihub._singleValue')`
		`+ def test_add_host_exists(self, _singleValue, get_host, _dml):`
		`+ get_host.return_value = {'id': 123}`
		`+ with self.assertRaises(koji.GenericError):`
		`+ self.exports.addHost('hostname', ['i386', 'x86_64'])`
		`+ _dml.assert_not_called()`
		`+ get_host.assert_called_once_with('hostname')`
		`+ _singleValue.assert_not_called()`
		`+`
		`+ @mock.patch('kojihub._dml')`
		`+ @mock.patch('kojihub.get_host')`
		`+ @mock.patch('kojihub._singleValue')`
		`+ def test_add_host_valid(self, _singleValue, get_host, _dml):`
		`+ get_host.return_value = {}`
		`+ _singleValue.side_effect = [333, 12]`
		`+ self.context.session.createUser.return_value = 456`
		`+`
		`+ r = self.exports.addHost('hostname', ['i386', 'x86_64'])`
		`+ self.assertEqual(r, 12)`
		`+`
		`+ self.context.session.assertPerm.assert_called_once_with('admin')`
		`+ kojihub.get_host.assert_called_once_with('hostname')`
		`+ self.context.session.createUser.assert_called_once_with('hostname',`
		`+ usertype=koji.USERTYPES['HOST'], krb_principal='-hostname-')`
		`+ self.assertEqual(_singleValue.call_count, 2)`
		`+ _singleValue.assert_has_calls([`
		`+ mock.call("SELECT id FROM channels WHERE name = 'default'"),`
		`+ mock.call("SELECT nextval('host_id_seq')", strict=True)`
		`+ ])`
		`+ self.assertEqual(_dml.call_count, 1)`
		`+ _dml.assert_called_once_with("INSERT INTO host (id, user_id, name) VALUES (%(hostID)i, %(userID)i, %(hostname)s)",`
		`+ {'hostID': 12, 'userID': 456, 'hostname': 'hostname'})`

tests/test_hub/test_add_host_to_channel.py

file added

+135

		`@@ -0,0 +1,135 @@`
		`+ import unittest`
		`+ import mock`
		`+`
		`+ import koji`
		`+ import kojihub`
		`+`
		`+ IP = kojihub.InsertProcessor`
		`+`
		`+`
		`+ class TestAddHostToChannel(unittest.TestCase):`
		`+ def getInsert(self, args, *kwargs):`
		`+ insert = IP(args, *kwargs)`
		`+ insert.execute = mock.MagicMock()`
		`+ self.inserts.append(insert)`
		`+ return insert`
		`+`
		`+ def setUp(self):`
		`+ self.InsertProcessor = mock.patch('kojihub.InsertProcessor',`
		`+ side_effect=self.getInsert).start()`
		`+ self.inserts = []`
		`+ self.context = mock.patch('kojihub.context').start()`
		`+ # It seems MagicMock will not automatically handle attributes that`
		`+ # start with "assert"`
		`+ self.context.session.assertLogin = mock.MagicMock()`
		`+ self.context.session.assertPerm = mock.MagicMock()`
		`+ self.context.event_id = 42`
		`+ self.context.session.user_id = 23`
		`+ self.context.opts = {'HostPrincipalFormat': '-%s-'}`
		`+ self.exports = kojihub.RootExports()`
		`+`
		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`+ @mock.patch('kojihub.list_channels')`
		`+ @mock.patch('kojihub.get_channel_id')`
		`+ @mock.patch('kojihub.get_host')`
		`+ def test_valid(self, get_host, get_channel_id, list_channels):`
		`+ name = 'hostname'`
		`+ cname = 'channel_name'`
		`+ get_host.return_value = {'id': 123, 'name': name}`
		`+ get_channel_id.return_value = 456`
		`+ list_channels.return_value = [{'id': 1, 'name': 'default'}]`
		`+`
		`+ kojihub.add_host_to_channel(name, cname, create=False)`
		`+`
		`+ get_host.assert_called_once_with(name)`
		`+ get_channel_id.assert_called_once_with(cname, create=False)`
		`+ list_channels.assert_called_once_with(123)`
		`+`
		`+ self.assertEqual(len(self.inserts), 1)`
		`+ insert = self.inserts[0]`
		`+ data = {`
		`+ 'host_id': 123,`
		`+ 'channel_id': 456,`
		`+ 'creator_id': 23,`
		`+ 'create_event': 42,`
		`+ }`
		`+ self.assertEqual(insert.table, 'host_channels')`
		`+ self.assertEqual(insert.data, data)`
		`+ self.assertEqual(insert.rawdata, {})`
		`+`
		`+ @mock.patch('kojihub.list_channels')`
		`+ @mock.patch('kojihub.get_channel_id')`
		`+ @mock.patch('kojihub.get_host')`
		`+ def test_no_host(self, get_host, get_channel_id, list_channels):`
		`+ name = 'hostname'`
		`+ cname = 'channel_name'`
		`+ get_host.return_value = None`
		`+`
		`+ with self.assertRaises(koji.GenericError):`
		`+ kojihub.add_host_to_channel(name, cname, create=False)`
		`+`
		`+ get_host.assert_called_once_with(name)`
		`+ self.assertEqual(len(self.inserts), 0)`
		`+`
		`+ @mock.patch('kojihub.get_channel_id')`
		`+ @mock.patch('kojihub.get_host')`
		`+ def test_no_channel(self, get_host, get_channel_id):`
		`+ name = 'hostname'`
		`+ cname = 'channel_name'`
		`+ get_host.return_value = {'id': 123, 'name': name}`
		`+ get_channel_id.return_value = None`
		`+`
		`+ with self.assertRaises(koji.GenericError):`
		`+ kojihub.add_host_to_channel(name, cname, create=False)`
		`+`
		`+ get_host.assert_called_once_with(name)`
		`+ get_channel_id.assert_called_once_with(cname, create=False)`
		`+ self.assertEqual(len(self.inserts), 0)`
		`+`
		`+ @mock.patch('kojihub.list_channels')`
		`+ @mock.patch('kojihub.get_channel_id')`
		`+ @mock.patch('kojihub.get_host')`
		`+ def test_no_channel_create(self, get_host, get_channel_id, list_channels):`
		`+ name = 'hostname'`
		`+ cname = 'channel_name'`
		`+ get_host.return_value = {'id': 123, 'name': name}`
		`+ get_channel_id.return_value = 456`
		`+ list_channels.return_value = [{'id': 1, 'name': 'default'}]`
		`+`
		`+ kojihub.add_host_to_channel(name, cname, create=True)`
		`+`
		`+ get_host.assert_called_once_with(name)`
		`+ get_channel_id.assert_called_once_with(cname, create=True)`
		`+ list_channels.assert_called_once_with(123)`
		`+`
		`+ self.assertEqual(len(self.inserts), 1)`
		`+ insert = self.inserts[0]`
		`+ data = {`
		`+ 'host_id': 123,`
		`+ 'channel_id': 456,`
		`+ 'creator_id': 23,`
		`+ 'create_event': 42,`
		`+ }`
		`+ self.assertEqual(insert.table, 'host_channels')`
		`+ self.assertEqual(insert.data, data)`
		`+ self.assertEqual(insert.rawdata, {})`
		`+`
		`+ @mock.patch('kojihub.list_channels')`
		`+ @mock.patch('kojihub.get_channel_id')`
		`+ @mock.patch('kojihub.get_host')`
		`+ def test_exists(self, get_host, get_channel_id, list_channels):`
		`+ name = 'hostname'`
		`+ cname = 'channel_name'`
		`+ get_host.return_value = {'id': 123, 'name': name}`
		`+ get_channel_id.return_value = 456`
		`+ list_channels.return_value = [{'id': 456, 'name': cname}]`
		`+`
		`+ with self.assertRaises(koji.GenericError):`
		`+ kojihub.add_host_to_channel(name, cname, create=False)`
		`+`
		`+ get_host.assert_called_once_with(name)`
		`+ get_channel_id.assert_called_once_with(cname, create=False)`
		`+ list_channels.assert_called_once_with(123)`
		`+ self.assertEqual(len(self.inserts), 0)`

tests/test_hub/test_edit_host.py

file added

+127

		`@@ -0,0 +1,127 @@`
		`+ import unittest`
		`+ import mock`
		`+`
		`+ import koji`
		`+ import kojihub`
		`+`
		`+ UP = kojihub.UpdateProcessor`
		`+ IP = kojihub.InsertProcessor`
		`+`
		`+`
		`+ class TestSetHostEnabled(unittest.TestCase):`
		`+ def getInsert(self, args, *kwargs):`
		`+ insert = IP(args, *kwargs)`
		`+ insert.execute = mock.MagicMock()`
		`+ self.inserts.append(insert)`
		`+ return insert`
		`+`
		`+ def getUpdate(self, args, *kwargs):`
		`+ update = UP(args, *kwargs)`
		`+ update.execute = mock.MagicMock()`
		`+ self.updates.append(update)`
		`+ return update`
		`+`
		`+ def setUp(self):`
		`+ self.InsertProcessor = mock.patch('kojihub.InsertProcessor',`
		`+ side_effect=self.getInsert).start()`
		`+ self.inserts = []`
		`+ self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor',`
		`+ side_effect=self.getUpdate).start()`
		`+ self.updates = []`
		`+ self.context = mock.patch('kojihub.context').start()`
		`+ # It seems MagicMock will not automatically handle attributes that`
		`+ # start with "assert"`
		`+ self.context.session.assertLogin = mock.MagicMock()`
		`+ self.context.session.assertPerm = mock.MagicMock()`
		`+ self.exports = kojihub.RootExports()`
		`+`
		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`+ def test_edit_host_missing(self):`
		`+ kojihub.get_host = mock.MagicMock()`
		`+ kojihub.get_host.side_effect = koji.GenericError`
		`+ with self.assertRaises(koji.GenericError):`
		`+ self.exports.editHost('hostname')`
		`+ kojihub.get_host.assert_called_once_with('hostname', strict=True)`
		`+ self.assertEqual(self.inserts, [])`
		`+ self.assertEqual(self.updates, [])`
		`+`
		`+ def test_edit_host_valid(self):`
		`+ kojihub.get_host = mock.MagicMock()`
		`+ kojihub.get_host.return_value = {`
		`+ 'id': 123,`
		`+ 'user_id': 234,`
		`+ 'name': 'hostname',`
		`+ 'arches': ['x86_64'],`
		`+ 'capacity': 100.0,`
		`+ 'description': 'description',`
		`+ 'comment': 'comment',`
		`+ 'enabled': False,`
		`+ }`
		`+ self.context.event_id = 42`
		`+ self.context.session.user_id = 23`
		`+`
		`+ r = self.exports.editHost('hostname', arches=['x86_64', 'i386'],`
		`+ capacity=12.0, comment='comment_new', non_existing_kw='bogus')`
		`+`
		`+ self.assertTrue(r)`
		`+ kojihub.get_host.assert_called_once_with('hostname', strict=True)`
		`+`
		`+ # revoke`
		`+ self.assertEqual(len(self.updates), 1)`
		`+ values = kojihub.get_host.return_value`
		`+ clauses = ['host_id = %(id)i', 'active = TRUE']`
		`+ revoke_data = {`
		`+ 'revoke_event': 42,`
		`+ 'revoker_id': 23`
		`+ }`
		`+ revoke_rawdata = {'active': 'NULL'}`
		`+ update = self.updates[0]`
		`+ self.assertEqual(update.table, 'host_config')`
		`+ self.assertEqual(update.values, values)`
		`+ self.assertEqual(update.clauses, clauses)`
		`+ self.assertEqual(update.data, revoke_data)`
		`+ self.assertEqual(update.rawdata, revoke_rawdata)`
		`+`
		`+ # insert`
		`+ self.assertEqual(len(self.inserts), 1)`
		`+ insert = self.inserts[0]`
		`+ #data = kojihub.get_host.return_value`
		`+ data = {`
		`+ 'create_event': 42,`
		`+ 'creator_id': 23,`
		`+ 'host_id': 123,`
		`+ 'arches': ['x86_64', 'i386'],`
		`+ 'capacity': 12.0,`
		`+ 'comment': 'comment_new',`
		`+ 'description': 'description',`
		`+ 'enabled': False,`
		`+ }`
		`+ rawdata = {}`
		`+ self.assertEqual(insert.table, 'host_config')`
		`+ self.assertEqual(insert.data, data)`
		`+ self.assertEqual(insert.rawdata, rawdata)`
		`+`
		`+ def test_edit_host_no_change(self):`
		`+ kojihub.get_host = mock.MagicMock()`
		`+ kojihub.get_host.return_value = {`
		`+ 'id': 123,`
		`+ 'user_id': 234,`
		`+ 'name': 'hostname',`
		`+ 'arches': ['x86_64'],`
		`+ 'capacity': 100.0,`
		`+ 'description': 'description',`
		`+ 'comment': 'comment',`
		`+ 'enabled': False,`
		`+ }`
		`+ self.context.event_id = 42`
		`+ self.context.session.user_id = 23`
		`+`
		`+ r = self.exports.editHost('hostname')`
		`+`
		`+ self.assertFalse(r)`
		`+ kojihub.get_host.assert_called_once_with('hostname', strict=True)`
		`+`
		`+ self.assertEqual(len(self.updates), 0)`
		`+ self.assertEqual(len(self.inserts), 0)`

tests/test_hub/test_get_host.py

file added

+95

		`@@ -0,0 +1,95 @@`
		`+ import unittest`
		`+ import mock`
		`+`
		`+ import koji`
		`+ import kojihub`
		`+`
		`+ QP = kojihub.QueryProcessor`
		`+`
		`+`
		`+ class TestSetHostEnabled(unittest.TestCase):`
		`+ def getQuery(self, args, *kwargs):`
		`+ query = QP(args, *kwargs)`
		`+ query.execute = mock.MagicMock()`
		`+ query.executeOne = mock.MagicMock()`
		`+ self.queries.append(query)`
		`+ return query`
		`+`
		`+ def setUp(self):`
		`+ self.QueryProcessor = mock.patch('kojihub.QueryProcessor',`
		`+ side_effect=self.getQuery).start()`
		`+ self.queries = []`
		`+ self.context = mock.patch('kojihub.context').start()`
		`+ # It seems MagicMock will not automatically handle attributes that`
		`+ # start with "assert"`
		`+ self.exports = kojihub.RootExports()`
		`+`
		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`+ def test_get_host_by_name(self):`
		`+ self.exports.getHost('hostname')`
		`+`
		`+ self.assertEqual(len(self.queries), 1)`
		`+ query = self.queries[0]`
		`+ columns = ['host.id', 'host.user_id', 'host.name', 'host.ready',`
		`+ 'host.task_load', 'host_config.arches',`
		`+ 'host_config.capacity', 'host_config.description',`
		`+ 'host_config.comment', 'host_config.enabled']`
		`+ joins = ['host ON host.id = host_config.host_id']`
		`+ aliases = ['id', 'user_id', 'name', 'ready', 'task_load',`
		`+ 'arches', 'capacity', 'description', 'comment', 'enabled']`
		`+ clauses = ['(host_config.active = TRUE)', 'host.name = %(hostInfo)s']`
		`+ values = {'hostInfo': 'hostname'}`
		`+ self.assertEqual(query.tables, ['host_config'])`
		`+ self.assertEqual(query.joins, joins)`
		`+ self.assertEqual(set(query.columns), set(columns))`
		`+ self.assertEqual(set(query.aliases), set(aliases))`
		`+ self.assertEqual(query.clauses, clauses)`
		`+ self.assertEqual(query.values, values)`
		`+`
		`+ def test_get_host_by_id_event(self):`
		`+ self.exports.getHost(123, event=345)`
		`+`
		`+ self.assertEqual(len(self.queries), 1)`
		`+ query = self.queries[0]`
		`+ columns = ['host.id', 'host.user_id', 'host.name', 'host.ready',`
		`+ 'host.task_load', 'host_config.arches',`
		`+ 'host_config.capacity', 'host_config.description',`
		`+ 'host_config.comment', 'host_config.enabled']`
		`+ joins = ['host ON host.id = host_config.host_id']`
		`+ aliases = ['id', 'user_id', 'name', 'ready', 'task_load',`
		`+ 'arches', 'capacity', 'description', 'comment', 'enabled']`
		`+ clauses = ['(host_config.create_event <= 345 AND ( host_config.revoke_event IS NULL OR 345 < host_config.revoke_event ))',`
		`+ 'host.id = %(hostInfo)i']`
		`+ values = {'hostInfo': 123}`
		`+ self.assertEqual(query.tables, ['host_config'])`
		`+ self.assertEqual(query.joins, joins)`
		`+ self.assertEqual(set(query.columns), set(columns))`
		`+ self.assertEqual(set(query.aliases), set(aliases))`
		`+ self.assertEqual(query.clauses, clauses)`
		`+ self.assertEqual(query.values, values)`
		`+`
		`+ def getQueryMissing(self, args, *kwargs):`
		`+ q = self.getQuery(args, *kwargs)`
		`+ q.executeOne.return_value = []`
		`+ return q`
		`+`
		`+ def test_get_host_missing(self):`
		`+ self.QueryProcessor.side_effect = self.getQueryMissing`
		`+`
		`+ r = self.exports.getHost(123)`
		`+ self.assertEqual(r, None)`
		`+`
		`+ with self.assertRaises(koji.GenericError):`
		`+ self.exports.getHost(123, strict=True)`
		`+`
		`+ self.assertEqual(len(self.queries), 2)`
		`+`
		`+ self.QueryProcessor.side_effect = self.getQuery`
		`+`
		`+ def test_get_host_invalid_hostinfo(self):`
		`+ with self.assertRaises(koji.GenericError):`
		`+ self.exports.getHost({'host_id': 567})`
		`+`
		`+ self.assertEqual(len(self.queries), 0)`

tests/test_hub/test_list_channels.py

file added

+79

		`@@ -0,0 +1,79 @@`
		`+ import unittest`
		`+ import mock`
		`+`
		`+ import koji`
		`+ import kojihub`
		`+`
		`+ QP = kojihub.QueryProcessor`
		`+`
		`+`
		`+ class TestListChannels(unittest.TestCase):`
		`+ def getQuery(self, args, *kwargs):`
		`+ query = QP(args, *kwargs)`
		`+ query.execute = mock.MagicMock()`
		`+ query.executeOne = mock.MagicMock()`
		`+ self.queries.append(query)`
		`+ return query`
		`+`
		`+ def setUp(self):`
		`+ self.QueryProcessor = mock.patch('kojihub.QueryProcessor',`
		`+ side_effect=self.getQuery).start()`
		`+ self.queries = []`
		`+ self.context = mock.patch('kojihub.context').start()`
		`+ # It seems MagicMock will not automatically handle attributes that`
		`+ # start with "assert"`
		`+ self.exports = kojihub.RootExports()`
		`+`
		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`+`
		`+ def test_all(self):`
		`+ kojihub.list_channels()`
		`+ self.assertEqual(len(self.queries), 1)`
		`+ query = self.queries[0]`
		`+ self.assertEqual(query.tables, ['channels'])`
		`+ self.assertEqual(query.aliases, ('name', 'id'))`
		`+ self.assertEqual(query.joins, None)`
		`+ self.assertEqual(query.values, {})`
		`+ self.assertEqual(query.columns, ('channels.name', 'channels.id'))`
		`+ self.assertEqual(query.clauses, None)`
		`+`
		`+ def test_host(self):`
		`+ kojihub.list_channels(hostID=1234)`
		`+`
		`+ self.assertEqual(len(self.queries), 1)`
		`+ query = self.queries[0]`
		`+ joins = ['channels ON channels.id = host_channels.channel_id']`
		`+ clauses = [`
		`+ '(host_channels.active = TRUE)',`
		`+ 'host_channels.host_id = %(host_id)s'`
		`+ ]`
		`+ self.assertEqual(query.tables, ['host_channels'])`
		`+ self.assertEqual(query.aliases, ('name', 'id'))`
		`+ self.assertEqual(query.joins, joins)`
		`+ self.assertEqual(query.values, {'host_id': 1234})`
		`+ self.assertEqual(query.columns, ('channels.name', 'channels.id'))`
		`+ self.assertEqual(query.clauses, clauses)`
		`+`
		`+ def test_host_and_event(self):`
		`+ kojihub.list_channels(hostID=1234, event=2345)`
		`+`
		`+ self.assertEqual(len(self.queries), 1)`
		`+ query = self.queries[0]`
		`+ joins = ['channels ON channels.id = host_channels.channel_id']`
		`+ clauses = [`
		`+ '(host_channels.create_event <= 2345 AND ( host_channels.revoke_event IS NULL OR 2345 < host_channels.revoke_event ))',`
		`+ 'host_channels.host_id = %(host_id)s',`
		`+ ]`
		`+ self.assertEqual(query.tables, ['host_channels'])`
		`+ self.assertEqual(query.aliases, ('name', 'id'))`
		`+ self.assertEqual(query.joins, joins)`
		`+ self.assertEqual(query.values, {'host_id': 1234})`
		`+ self.assertEqual(query.columns, ('channels.name', 'channels.id'))`
		`+ self.assertEqual(query.clauses, clauses)`
		`+`
		`+ def test_event_only(self):`
		`+ with self.assertRaises(koji.GenericError):`
		`+ kojihub.list_channels(event=1234)`
		`+ self.assertEqual(len(self.queries), 0)`

tests/test_hub/test_list_hosts.py

file modified

+32 -27

		`@@ -29,9 +29,9 @@`

		`self.assertEqual(len(self.queries), 1)`
		`query = self.queries[0]`
		`- self.assertEqual(query.tables, ['host'])`
		`- self.assertEqual(query.joins, [])`
		`- self.assertEqual(query.clauses, [])`
		`+ self.assertEqual(query.tables, ['host_config'])`
		`+ self.assertEqual(query.joins, ['host ON host.id = host_config.host_id'])`
		`+ self.assertEqual(query.clauses, ['host_config.active IS TRUE',])`

		`@mock.patch('kojihub.get_user')`
		`def test_list_hosts_user_id(self, get_user):`
		`@@ -40,9 +40,9 @@`

		`self.assertEqual(len(self.queries), 1)`
		`query = self.queries[0]`
		`- self.assertEqual(query.tables, ['host'])`
		`- self.assertEqual(query.joins, [])`
		`- self.assertEqual(query.clauses, ['user_id = %(userID)i'])`
		`+ self.assertEqual(query.tables, ['host_config'])`
		`+ self.assertEqual(query.joins, ['host ON host.id = host_config.host_id'])`
		`+ self.assertEqual(query.clauses, ['host_config.active IS TRUE', 'user_id = %(userID)i'])`

		`@mock.patch('kojihub.get_channel_id')`
		`def test_list_hosts_channel_id(self, get_channel_id):`
		`@@ -51,27 +51,32 @@`

		`self.assertEqual(len(self.queries), 1)`
		`query = self.queries[0]`
		`- self.assertEqual(query.tables, ['host'])`
		`- self.assertEqual(query.joins, ['host_channels on host.id = host_channels.host_id'])`
		`- self.assertEqual(query.clauses, ['host_channels.channel_id = %(channelID)i'])`
		`+ self.assertEqual(query.tables, ['host_config'])`
		`+ self.assertEqual(query.joins, ['host ON host.id = host_config.host_id',`
		`+ 'host_channels ON host.id = host_channels.host_id'])`
		`+ self.assertEqual(query.clauses, [`
		`+ 'host_config.active IS TRUE',`
		`+ 'host_channels.channel_id = %(channelID)i',`
		`+ 'host_channels.active IS TRUE',`
		`+ ])`

		`def test_list_hosts_single_arch(self):`
		`self.exports.listHosts(arches='x86_64')`

		`self.assertEqual(len(self.queries), 1)`
		`query = self.queries[0]`
		`- self.assertEqual(query.tables, ['host'])`
		`- self.assertEqual(query.joins, [])`
		`- self.assertEqual(query.clauses, [r"""(arches ~ E'\\mx86_64\\M')"""])`
		`+ self.assertEqual(query.tables, ['host_config'])`
		`+ self.assertEqual(query.joins, ['host ON host.id = host_config.host_id'])`
		`+ self.assertEqual(query.clauses, ['host_config.active IS TRUE',r"""(arches ~ E'\\mx86_64\\M')"""])`

		`def test_list_hosts_multi_arch(self):`
		`self.exports.listHosts(arches=['x86_64', 's390'])`

		`self.assertEqual(len(self.queries), 1)`
		`query = self.queries[0]`
		`- self.assertEqual(query.tables, ['host'])`
		`- self.assertEqual(query.joins, [])`
		`- self.assertEqual(query.clauses, [r"""(arches ~ E'\\mx86_64\\M' OR arches ~ E'\\ms390\\M')"""])`
		`+ self.assertEqual(query.tables, ['host_config'])`
		`+ self.assertEqual(query.joins, ['host ON host.id = host_config.host_id'])`
		`+ self.assertEqual(query.clauses, ['host_config.active IS TRUE',r"""(arches ~ E'\\mx86_64\\M' OR arches ~ E'\\ms390\\M')"""])`

		`def test_list_hosts_bad_arch(self):`
		`with self.assertRaises(koji.GenericError):`
		`@@ -82,33 +87,33 @@`

		`self.assertEqual(len(self.queries), 1)`
		`query = self.queries[0]`
		`- self.assertEqual(query.tables, ['host'])`
		`- self.assertEqual(query.joins, [])`
		`- self.assertEqual(query.clauses, ['ready is true'])`
		`+ self.assertEqual(query.tables, ['host_config'])`
		`+ self.assertEqual(query.joins, ['host ON host.id = host_config.host_id'])`
		`+ self.assertEqual(query.clauses, ['host_config.active IS TRUE','ready IS TRUE'])`

		`def test_list_hosts_nonready(self):`
		`self.exports.listHosts(ready=0)`

		`self.assertEqual(len(self.queries), 1)`
		`query = self.queries[0]`
		`- self.assertEqual(query.tables, ['host'])`
		`- self.assertEqual(query.joins, [])`
		`- self.assertEqual(query.clauses, ['ready is false'])`
		`+ self.assertEqual(query.tables, ['host_config'])`
		`+ self.assertEqual(query.joins, ['host ON host.id = host_config.host_id'])`
		`+ self.assertEqual(query.clauses, ['host_config.active IS TRUE','ready IS FALSE'])`

		`def test_list_hosts_enabled(self):`
		`self.exports.listHosts(enabled=1)`

		`self.assertEqual(len(self.queries), 1)`
		`query = self.queries[0]`
		`- self.assertEqual(query.tables, ['host'])`
		`- self.assertEqual(query.joins, [])`
		`- self.assertEqual(query.clauses, ['enabled is true'])`
		`+ self.assertEqual(query.tables, ['host_config'])`
		`+ self.assertEqual(query.joins, ['host ON host.id = host_config.host_id'])`
		`+ self.assertEqual(query.clauses, ['host_config.active IS TRUE','enabled IS TRUE'])`

		`def test_list_hosts_disabled(self):`
		`self.exports.listHosts(enabled=0)`

		`self.assertEqual(len(self.queries), 1)`
		`query = self.queries[0]`
		`- self.assertEqual(query.tables, ['host'])`
		`- self.assertEqual(query.joins, [])`
		`- self.assertEqual(query.clauses, ['enabled is false'])`
		`+ self.assertEqual(query.tables, ['host_config'])`
		`+ self.assertEqual(query.joins, ['host ON host.id = host_config.host_id'])`
		`+ self.assertEqual(query.clauses, ['host_config.active IS TRUE','enabled IS FALSE'])`

tests/test_hub/test_remove_host_from_channel.py

file added

+103

		`@@ -0,0 +1,103 @@`
		`+ import unittest`
		`+ import mock`
		`+`
		`+ import koji`
		`+ import kojihub`
		`+`
		`+ UP = kojihub.UpdateProcessor`
		`+`
		`+`
		`+ class TestRemoveHostFromChannel(unittest.TestCase):`
		`+ def getUpdate(self, args, *kwargs):`
		`+ update = UP(args, *kwargs)`
		`+ update.execute = mock.MagicMock()`
		`+ self.updates.append(update)`
		`+ return update`
		`+`
		`+ def setUp(self):`
		`+ self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor',`
		`+ side_effect=self.getUpdate).start()`
		`+ self.updates = []`
		`+ self.context = mock.patch('kojihub.context').start()`
		`+ # It seems MagicMock will not automatically handle attributes that`
		`+ # start with "assert"`
		`+ self.context.session.assertLogin = mock.MagicMock()`
		`+ self.context.session.assertPerm = mock.MagicMock()`
		`+ self.context.event_id = 42`
		`+ self.context.session.user_id = 23`
		`+ self.context.opts = {'HostPrincipalFormat': '-%s-'}`
		`+ self.exports = kojihub.RootExports()`
		`+`
		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`+ @mock.patch('kojihub.list_channels')`
		`+ @mock.patch('kojihub.get_channel_id')`
		`+ @mock.patch('kojihub.get_host')`
		`+ def test_valid(self, get_host, get_channel_id, list_channels):`
		`+ get_host.return_value = {'id': 123, 'name': 'hostname'}`
		`+ get_channel_id.return_value = 234`
		`+ list_channels.return_value = [{'id': 234, 'name': 'channelname'}]`
		`+`
		`+ kojihub.remove_host_from_channel('hostname', 'channelname')`
		`+`
		`+ get_host.assert_called_once_with('hostname')`
		`+ get_channel_id.assert_called_once_with('channelname')`
		`+ list_channels.assert_called_once_with(123)`
		`+`
		`+ self.assertEqual(len(self.updates), 1)`
		`+ update = self.updates[0]`
		`+ values = {`
		`+ 'host_id': 123,`
		`+ 'channel_id': 234,`
		`+ }`
		`+ clauses = [`
		`+ 'host_id = %(host_id)i AND channel_id = %(channel_id)i',`
		`+ 'active = TRUE',`
		`+ ]`
		`+ self.assertEqual(update.table, 'host_channels')`
		`+ self.assertEqual(update.values, values)`
		`+ self.assertEqual(update.clauses, clauses)`
		`+`
		`+ @mock.patch('kojihub.list_channels')`
		`+ @mock.patch('kojihub.get_channel_id')`
		`+ @mock.patch('kojihub.get_host')`
		`+ def test_wrong_host(self, get_host, get_channel_id, list_channels):`
		`+ get_host.return_value = None`
		`+`
		`+ with self.assertRaises(koji.GenericError):`
		`+ kojihub.remove_host_from_channel('hostname', 'channelname')`
		`+`
		`+ get_host.assert_called_once_with('hostname')`
		`+ self.assertEqual(len(self.updates), 0)`
		`+`
		`+ @mock.patch('kojihub.list_channels')`
		`+ @mock.patch('kojihub.get_channel_id')`
		`+ @mock.patch('kojihub.get_host')`
		`+ def test_wrong_channel(self, get_host, get_channel_id, list_channels):`
		`+ get_host.return_value = {'id': 123, 'name': 'hostname'}`
		`+ get_channel_id.return_value = None`
		`+ list_channels.return_value = [{'id': 234, 'name': 'channelname'}]`
		`+`
		`+ with self.assertRaises(koji.GenericError):`
		`+ kojihub.remove_host_from_channel('hostname', 'channelname')`
		`+`
		`+ get_host.assert_called_once_with('hostname')`
		`+ get_channel_id.assert_called_once_with('channelname')`
		`+ self.assertEqual(len(self.updates), 0)`
		`+`
		`+ @mock.patch('kojihub.list_channels')`
		`+ @mock.patch('kojihub.get_channel_id')`
		`+ @mock.patch('kojihub.get_host')`
		`+ def test_missing_record(self, get_host, get_channel_id, list_channels):`
		`+ get_host.return_value = {'id': 123, 'name': 'hostname'}`
		`+ get_channel_id.return_value = 234`
		`+ list_channels.return_value = []`
		`+`
		`+ with self.assertRaises(koji.GenericError):`
		`+ kojihub.remove_host_from_channel('hostname', 'channelname')`
		`+`
		`+ get_host.assert_called_once_with('hostname')`
		`+ get_channel_id.assert_called_once_with('channelname')`
		`+ list_channels.assert_called_once_with(123)`
		`+ self.assertEqual(len(self.updates), 0)`

tests/test_hub/test_set_host_enabled.py

file added

+147

		`@@ -0,0 +1,147 @@`
		`+ import unittest`
		`+ import mock`
		`+`
		`+ import koji`
		`+ import kojihub`
		`+`
		`+ UP = kojihub.UpdateProcessor`
		`+ IP = kojihub.InsertProcessor`
		`+`
		`+`
		`+ class TestSetHostEnabled(unittest.TestCase):`
		`+ def getInsert(self, args, *kwargs):`
		`+ insert = IP(args, *kwargs)`
		`+ insert.execute = mock.MagicMock()`
		`+ self.inserts.append(insert)`
		`+ return insert`
		`+`
		`+ def getUpdate(self, args, *kwargs):`
		`+ update = UP(args, *kwargs)`
		`+ update.execute = mock.MagicMock()`
		`+ self.updates.append(update)`
		`+ return update`
		`+`
		`+ def setUp(self):`
		`+ self.InsertProcessor = mock.patch('kojihub.InsertProcessor',`
		`+ side_effect=self.getInsert).start()`
		`+ self.inserts = []`
		`+ self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor',`
		`+ side_effect=self.getUpdate).start()`
		`+ self.updates = []`
		`+ self.context = mock.patch('kojihub.context').start()`
		`+ # It seems MagicMock will not automatically handle attributes that`
		`+ # start with "assert"`
		`+ self.context.session.assertLogin = mock.MagicMock()`
		`+ self.context.session.assertPerm = mock.MagicMock()`
		`+ self.exports = kojihub.RootExports()`
		`+`
		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`+ def test_enableHost_missing(self):`
		`+ # non-existing hostname`
		`+ kojihub.get_host = mock.MagicMock()`
		`+ kojihub.get_host.return_value = {}`
		`+ with self.assertRaises(koji.GenericError):`
		`+ self.exports.enableHost('hostname')`
		`+ self.assertEqual(self.updates, [])`
		`+ self.assertEqual(self.inserts, [])`
		`+ kojihub.get_host.assert_called_once_with('hostname')`
		`+`
		`+ def test_enableHost_valid(self):`
		`+ kojihub.get_host = mock.MagicMock()`
		`+ kojihub.get_host.return_value = {`
		`+ 'id': 123,`
		`+ 'user_id': 234,`
		`+ 'name': 'hostname',`
		`+ 'arches': ['x86_64'],`
		`+ 'capacity': 100.0,`
		`+ 'description': 'description',`
		`+ 'comment': 'comment',`
		`+ 'enabled': False,`
		`+ }`
		`+ self.context.event_id = 42`
		`+ self.context.session.user_id = 23`
		`+`
		`+ self.exports.enableHost('hostname')`
		`+`
		`+ kojihub.get_host.assert_called_once_with('hostname')`
		`+ # revoke`
		`+ self.assertEqual(len(self.updates), 1)`
		`+ values = kojihub.get_host.return_value`
		`+ clauses = ['host_id = %(id)i', 'active = TRUE']`
		`+ revoke_data = {`
		`+ 'revoke_event': 42,`
		`+ 'revoker_id': 23`
		`+ }`
		`+ revoke_rawdata = {'active': 'NULL'}`
		`+ update = self.updates[0]`
		`+ self.assertEqual(update.table, 'host_config')`
		`+ self.assertEqual(update.values, values)`
		`+ self.assertEqual(update.clauses, clauses)`
		`+ self.assertEqual(update.data, revoke_data)`
		`+ self.assertEqual(update.rawdata, revoke_rawdata)`
		`+`
		`+ # insert`
		`+ insert = self.inserts[0]`
		`+ data = kojihub.get_host.return_value`
		`+ data['create_event'] = 42`
		`+ data['creator_id'] = 23`
		`+ data['enabled'] = True`
		`+ data['host_id'] = data['id']`
		`+ del data['id']`
		`+ rawdata = {}`
		`+ self.assertEqual(insert.table, 'host_config')`
		`+ self.assertEqual(insert.data, data)`
		`+ self.assertEqual(insert.rawdata, rawdata)`
		`+`
		`+ self.assertEqual(len(self.inserts), 1)`
		`+`
		`+ def test_disableHost_valid(self):`
		`+ kojihub.get_host = mock.MagicMock()`
		`+ kojihub.get_host.return_value = {`
		`+ 'id': 123,`
		`+ 'user_id': 234,`
		`+ 'name': 'hostname',`
		`+ 'arches': ['x86_64'],`
		`+ 'capacity': 100.0,`
		`+ 'description': 'description',`
		`+ 'comment': 'comment',`
		`+ 'enabled': True,`
		`+ }`
		`+ self.context.event_id = 42`
		`+ self.context.session.user_id = 23`
		`+`
		`+ self.exports.disableHost('hostname')`
		`+`
		`+ kojihub.get_host.assert_called_once_with('hostname')`
		`+ # revoke`
		`+ self.assertEqual(len(self.updates), 1)`
		`+ values = kojihub.get_host.return_value`
		`+ clauses = ['host_id = %(id)i', 'active = TRUE']`
		`+ revoke_data = {`
		`+ 'revoke_event': 42,`
		`+ 'revoker_id': 23`
		`+ }`
		`+ revoke_rawdata = {'active': 'NULL'}`
		`+ update = self.updates[0]`
		`+ self.assertEqual(update.table, 'host_config')`
		`+ self.assertEqual(update.values, values)`
		`+ self.assertEqual(update.clauses, clauses)`
		`+ self.assertEqual(update.data, revoke_data)`
		`+ self.assertEqual(update.rawdata, revoke_rawdata)`
		`+`
		`+ # insert`
		`+ insert = self.inserts[0]`
		`+ data = kojihub.get_host.return_value`
		`+ data['create_event'] = 42`
		`+ data['creator_id'] = 23`
		`+ data['enabled'] = False`
		`+ data['host_id'] = data['id']`
		`+ del data['id']`
		`+ rawdata = {}`
		`+ self.assertEqual(insert.table, 'host_config')`
		`+ self.assertEqual(insert.data, data)`
		`+ self.assertEqual(insert.rawdata, rawdata)`
		`+`
		`+ self.assertEqual(len(self.inserts), 1)`

tkopecek commented 6 years ago

Hosts now have history.

host table was split to host (containing ephemereal and non-editable
data (load, activity, name, user_id)) and host_config containing
data changeable by admins (archs, capacity, ...). This table is
versioned and searchable via queryHistory.

Fixes: https://pagure.io/koji/issue/638

1 new commit added

update schema.sql for host_config

6 years ago

2 new commits added

add index
JOIN fixes for host_config

6 years ago

mikem commented 6 years ago

Seems like we should also version host_channels.

mikem commented on line 3 of docs/schema-upgrade-1.15-1.16.sql 6 years ago

wrong version in comment

mikem commented 6 years ago

There are a lot of changes here, we should make sure we have unit test coverage.

tkopecek commented 6 years ago

If we add history to host_channels, it would affect remove_channel. In such case we will never delete most of channels as it would still be referenced from inactive host_channels. On the other hand, we can go one more step further and version also channel table.

Would it make sense to create host_channels/channels version another PR?

/added some tests meanwhile/

2 new commits added

host history tests
update version in comment

6 years ago

mikem commented 6 years ago

If we add history to host_channels, it would affect remove_channel. In such case we will never delete most of channels as it would still be referenced from inactive host_channels.

I think this will be ok. We'll just need to update the docstring. Having channel history for hosts will be worth the price.

On the other hand, we can go one more step further and version also channel table.

We don't have any other versioned name lookup tables, I'm not sure I'd want to have an exception to that here.

We could follow the approach with the tag table and have a versioned channel_config table. Nothing much to put there right now, but maybe in the future.

rebased onto 491de84102ec8b4d569bcbab70cae0cbf143b346

6 years ago

tkopecek commented 6 years ago

Added versions for host_channels.

rebased onto 9ee72d8

6 years ago

mikem commented 5 years ago

Looks good overall. Fixed a few issues and added support to query history by host and channel.

https://github.com/mikem23/koji-playground/commits/pagure/pr/778

mikem commented 5 years ago

Seems to be working :smile:

$ kdev lkoji list-history --host builder-04
2018-04-27 15:57:18,476 [DEBUG] koji: Opening new requests session
successfully connected to hub
Fri Apr 27 11:11:32 2018 host builder-04 added to channel default by mikem [still active]
Fri Apr 27 11:11:32 2018 new host: builder-04 by mikem
Fri Apr 27 11:22:17 2018 host configuration for builder-04 altered by mikem
Fri Apr 27 11:22:19 2018 host configuration for builder-04 altered by mikem
Fri Apr 27 11:22:25 2018 host configuration for builder-04 altered by mikem
    enabled: True -> False
Fri Apr 27 11:22:26 2018 host configuration for builder-04 altered by mikem
    enabled: False -> True
Fri Apr 27 11:22:27 2018 host configuration for builder-04 altered by mikem
    enabled: True -> False
Fri Apr 27 11:23:33 2018 host builder-04 added to channel image by mikem
Fri Apr 27 11:23:42 2018 host builder-04 removed from channel image by mikem
Fri Apr 27 11:23:43 2018 host builder-04 added to channel image by mikem
Fri Apr 27 11:23:45 2018 host builder-04 removed from channel image by mikem
Fri Apr 27 11:30:51 2018 host configuration for builder-04 altered by mikem
    comment: None -> hello
Fri Apr 27 11:30:55 2018 host configuration for builder-04 altered by mikem
    comment: hello -> hello world

mikem commented 5 years ago

~~err, hold, on, gotta fix that branch...~~

Edited 5 years ago by mikem

mikem commented 5 years ago

ok, fixed now

tkopecek commented 5 years ago

:thumbsup:

mikem commented 5 years ago

Commit 65d6990 fixes this pull-request