Next two lines use original info variable.

python3 doesn't know has_key, please use 'k in vars(self)' instead

Doesn't it make more sense to put these calls inside scm.checkout() ? It would be maintained only in one place and it will have access to SCM's scope. Not sure if it is not against original requirements.

fixed no1 and no2 problems from @tkopecek
About no3 problem, I think putting the calls in scm.checkout() is better too. I'm not sure why @xning write the calls around it, maybe he didn't want to modify scm.checkout() or scm.__init__()'s signature?
@mikem What's your opinion about it?

This is digging into some of the challenge and earlier debate. The callback will also want information from the task scope (e.g. is this a for a scratch build). There's no perfect answer.

I think putting the calls in scm.checkout() is better too

In order to do anything useful with these callbacks, we will need information about the task. That is not available in the SCM context.

It currently fails for me (rebased to master) with such command:
koji build --scratch f24 git://xyz#origin/random_branch

Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/koji/daemon.py", line 1166, in runTask
    response = (handler.run(),)
  File "/usr/lib/python2.7/site-packages/koji/tasks.py", line 158, in run
    return koji.util.call_with_argcheck(self.handler, self.params, self.opts)
 File "/usr/lib/python2.7/site-packages/koji/util.py", line 156, in call_with_argcheck
    return func(*args, **kwargs)
 File "/usr/sbin/kojid", line 4428, in handler
self.run_plugin('preSCMCheckout', scminfo=scm.get_info())
  File "/usr/lib/python2.7/site-packages/koji/daemon.py", line 236, in get_info
    return dslice(vars(self), keys)
 File "/usr/lib/python2.7/site-packages/koji/util.py", line 140, in dslice
   ret[key] = dict[key]
KeyError: 'path'

vars(self) in get_info contains:

{
    'repository': '/rpms/koji',
    'url': 'git://xyz#origin/random_branch',
    'scmtype': 'GIT',
    'module': '',
    'source_cmd': ['rhpkg'],
    'host': 'xyz',
    'user': None,
    'logger': <logging.Logger object at 0x7fc6c27dcdd0>,
    'scheme': 'git://',
    'use_common': False,
    'revision': 'origin/random_branch'
}

@tkopecek Thanks for your test.
Updated.

One more issue. How I'm supposed to register for this call? I've tried to create primitive builder plugin:

import logging
from koji.plugin import callback
@callback('preSCMCheckout')
def mycallback(*args, **kwargs):
    logging.debug(str(kwargs))

But these callbacks are never registered in builder (via register_callback). Registering happens only in hub. Is this the intended way? In such case, koji.daemon.scanPlugin / findHandlers needs to be extended to register also these.

Update the code to enable callback plugin for builder and vm.
example args are:
{'scminfo': {'repository': '/azhuzhu/simple-dist', 'url': 'git://github.com/azhuzhu/simple-dist?#62ed4a12adfbe273b100f069c39965b07cf54761', 'scmtype': 'GIT', 'module': '', 'host': 'github.com', 'user': None, 'scheme': 'git://', 'revision': '62ed4a12adfbe273b100f069c39965b07cf54761'}, 'taskinfo': {'weight': 1.0, 'parent': 198, 'completion_time': None, 'request': ['git://github.com/azhuzhu/simple-dist?#62ed4a12adfbe273b100f069c39965b07cf54761', 6, {'repo_id': 25}], 'start_time': '2017-04-25 02:02:33.478973', 'start_ts': 1493085753.4789701, 'state': 1, 'awaited': True, 'label': 'srpm', 'priority': 19, 'channel_id': 1, 'waiting': None, 'create_time': '2017-04-25 02:02:33.441804', 'id': 199, 'create_ts': 1493085753.4418001, 'owner': 1, 'host_id': 1, 'completion_ts': None, 'arch': 'noarch', 'method': 'buildSRPMFromSCM'}}

Here's no taginfo, but it could be got by
1. parent task's request in plugin
2. task handler and then pass it to callback, which could be passed from parent task.
1 is easier for coding.
2 seems more direct and efficient but complex

@mikem @tkopecek Any thought?

As query is possible, I would stay with option one for simplicity.
BTW - it works for me now correctly.

One more question - how I'm supposed to get e.g. reference to hub connection? Parsing builder config shouldn't be necessary.

Guess using context works for this situation.

parent = context.handlers.call('getTaskInfo', info['parent'], request=True)

this is an example from mavensign hub plugin.

Let me test this solution at first.

You have a reference to the build_tag dict here (and everywhere else we're calling this plugin, I believe). Seems like it would make sense to pass that to the callback as well.

@mikeb Maybe whole target info?

@mikeb Maybe whole target info?
@tkopecek Not every task has a reference to the build target. buildSRPMFromSCM only gets passed a build_tag, which could be associated with multiple targets.

For the SCM object in kojikamid.WindowsBuild, session instance cannot be got, so I'll loose the requirement of arguments of SCMCheckout plugin. build_tag, session, taskinfo, scratch will be pushed as mush as possible. I think it would work for current code.
Is there possibly any underlying problem here?

For the SCM object in kojikamid.WindowsBuild, session instance cannot be got, so I'll loose the requirement of arguments of SCMCheckout plugin. build_tag, session, taskinfo, scratch will be pushed as mush as possible. I think it would work for current code.

Find a solution here: invoking callback in kojikamid.WindowsBuild via remote call of VMExecTask

updated.
Here I don't control the type of build_tag, It might be a tagname or taginfo dict or even tagID, which should be checked in callback plugin.
session is used to get more information from hub.
Also pass scratch into postSCMCheckout callbacks.

I've removed the callback caller in win vm builder, since there's no requirement for winbuild.

I think this is about where we need to be. Just a couple things.

• let's call it run_callbacks instead of run_plugin
• callbacks should register themselves with the decorator, so I don't think we need to have that registerCallback call, but please let me know if I'm missing something
• since we're not changing kojikamid, maybe don't touch it at all?

callbacks should register themselves with the decorator, so I don't think we need to have that registerCallback call, but please let me know if I'm missing something

@mikem current callback decorator doesn't invoke koji.plugin.register_callback(), so if I didn't misunderstand your reply, I guess it's necessary to register callbacks in kojid, like what's done in kojixmlrpc. But, refactoring plugin.py to make all available plugins to be equipped would be better than current implementation I think. Would we done this within this feature?

current callback decorator doesn't invoke koji.plugin.register_callback(),

Hmm, I guess I am mistaken. I didn't realize the hub was also doing this.

add scratch param for 'preSCMCheckout' callbacks, too

ping @mikem - any more changes needed?

Commit 5bd1d332 fixes this pull-request