~~It looks like if a task_id is set by the reservation, but that a task_id is not given at import time, then:~~

1. ~~the import will not error~~
2. ~~the task_id will be set to NULL in update_build~~

~~This doesn't seem right. If a build is reserved with a task_id, then the import call ought to be consistent with that~~

EDIT: nevermind, I was reading the logic backwards

The new code checks for a task id with metadata['build'].get('task_id'), but the old PR (whose code is still below) uses more complex logic (checking multiple fields and casting to int). Should we support similar here? Even if we mean for the reservation case to be stricter, we would probably still want to check the other field for consistency.

Perhaps a get_task_id_from_metadata function that could be used both places? Or actually, maybe just move calculate that value earlier before the reservation check.

Not from this PR, but we probably want to complain if both task_id and container_koji_task_id are given and do not match.

We have buildinfo = get_build(build_id, strict=True) and then we check if 'task_id' in buildinfo. However, get_build will always return a task_id field, it just might be None. So we probably want to check for non-None here instead.

In the second case, the return will be None. I think you want to have return task_id or task_id2.

Not introduced here, but I wonder if these int casts should use convert_value for sane errors on bad input.

Also, it reads a little wrong to call the function get_task_info_from_metadata, but only pass in the part of the metadata. Might be saner to pass in metadata. In any case, using binfo as the parameter makes it sound like we could be expecting a get_build result.

            if (buildinfo['task_id'] or task_id) and buildinfo['task_id'] != task_id:
                raise koji.GenericError('Build is owned by task %(task_id)s' % buildinfo)

This condition seems wrong. If the existing build entry has no task id, then we want to allow the final import to set one (a key case in the motivating issue).

I think you want

            if buildinfo['task_id']  and buildinfo['task_id'] != task_id:
                raise koji.GenericError('Build is owned by task %(task_id)s' % buildinfo)

lgtm

Commit 100411c1 fixes this pull-request