#319 Added support for CG provided owner
Merged by mikem. Opened by breilly.
breilly/koji cgprovidesowner299  into  master

Download 319.patch

Allows content generators to set their own owner for the build through build metadata.

Session owner will be used otherwise.

Waiting for @mikem , but if he will agree with this API extension, we will need also to fix documentation (and provide few unittests).

Is this line necessary? Seems redundant since we set buildinfo['owner'] below.

rebased

I'm fine with using ['build']['owner'].

However, we should probably require a specific format here. I think simply username is going to be the simplest and sanest.

@mikem should that just be specified in documentation, or should koji also check if ['build']['owner'] is a string?

Should keep the logic together and use the strict option. E.g. something like

owner = metadata['build'].get('owner', None)
if owner:
    if not isinstance(owner, basestring):
        raise ....
    buildinfo['owner'] = get_user(owner, strict=True)['id']

rebased

Seems to work just fine. Though testing this made me realize that the strict option in get_user() wasn't working. Filed #321 to fix

@breilly, can you also update the unit test so get coverage on the "if owner:" block?

@mikem I added an owner field to the default.json used for cg tests: "owner": "koji". The test fails in get_user because the ['build']['owner'] string gets imported as unicode instead of str.

Should prep_build correct for this, or should get_user be changed to allow 'basestring' instead of just 'str'?

@breilly we may just want to pull in a change I wrote a while back. See PR #322

If I apply that along with this pr and the test tweak you wrote, then the test gets over the unicode bump and fails on: GenericError: No such user: 'koji'. That suggests we need to adjust some mocks in the test

Just merged #322, so if you rebase you will have it

rebased

I've updated the mocks - the unit tests now pass locally.

Works for me just fine. Last thing - can you extend content_generator_metadata.rst with docs on this new optional field, please?

rebased

rebased

Commit ef043e2c fixes this pull-request

Pull-Request has been merged by mikem@redhat.com

Metadata