Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tornado.gen does not work with callbacks that have kwargs #351

Closed
rgarcia opened this issue Sep 6, 2011 · 5 comments
Closed

tornado.gen does not work with callbacks that have kwargs #351

rgarcia opened this issue Sep 6, 2011 · 5 comments

Comments

@rgarcia
Copy link

rgarcia commented Sep 6, 2011

I'm playing around with tornado.gen, and it seems to croak on callbacks that are triggered with keyword arguments. Specifically, asyncmongo sends an "error" keyword argument to the callback you specify. The tornado.gen.Task class's callback() method isn't set up to handle this. Here is the relevant code:

class BaseHandler(tornado.web.RequestHandler):
    @property
    def db(self):
        if not hasattr(self, '_db'):
            self._db = asyncmongo.Client(pool_id='request_handlers',
                                             host=options.mongo_host,
                                             port=options.mongo_port,
                                             dbname=options.mongo_db)
        return self._db

class UserHandler(BaseHandler):
    @tornado.web.asynchronous
    @tornado.gen.engine
    def get(self,user_id):
        response = yield tornado.gen.Task(self.db.users.find,{'_id':user_id},limit=1)
        if len(response) != 1:
            self.set_status(400)
            self.write({'error': 'no user with this id'})
            return
        self.write(dict((k,response[0][k]) for \
                            k in [ 'first_name', 'last_name', 'email' ]))

the error: "TypeError: callback() got an unexpected keyword argument 'error'"

I'm hacking around trying to figure out a fix to this, but I still haven't wrapped my head around the gen.py code. I tried just adding a **kwargs argument to the callback() function, but that didn't work (the Runner loop timed out). this is similar to the hack I used to make this code work with swirl.

@rgarcia rgarcia closed this as completed Sep 6, 2011
@rgarcia rgarcia reopened this Sep 6, 2011
@rgarcia
Copy link
Author

rgarcia commented Sep 6, 2011

accidentally closed the issue, reopened it

@rgarcia
Copy link
Author

rgarcia commented Sep 6, 2011

actually, adding **kwargs to the callback() method does fix the problem, but does nothing to expose those keyword arguments.

    def callback(self, arg=None, **kwargs):
        self.runner.set_result(self.key, arg)

don't know the best way to expose them...

@bdarnell
Copy link
Member

bdarnell commented Sep 7, 2011

Yeah, the trick is that we can only pass a single object through the yield expression. The only way to handle all the possible argument scenarios would be to pass a (args, kwargs) tuple, but that's awkward to work with. Does asyncmongo ever give both positional and keyword arguments at the same time? If not, we could automatically detect no args, one positional arg, or no positional args with keyword arguments (but then kwargs would come through as a dictionary and be indistinguishable from one positional arg that happened to be a dict). The most general solution is to have several variants of gen.Callback depending on the expected signature.

@rgarcia
Copy link
Author

rgarcia commented Sep 7, 2011

I think asyncmongo always passes an 'error' kwarg, and the result is
passed as a positional argument. What if we detected kwargs and
returned (args,kwargs) in this case? The only problem is when the
callback signature is unknown/dependent on the result, but in that
case you should be prepared to check for kwargs anyway.

On Sep 7, 2011, at 12:54 AM, bdarnell
reply@reply.github.com
wrote:

Yeah, the trick is that we can only pass a single object through the yield expression. The only way to handle all the possible argument scenarios would be to pass a (args, kwargs) tuple, but that's awkward to work with. Does asyncmongo ever give both positional and keyword arguments at the same time? If not, we could automatically detect no args, one positional arg, or no positional args with keyword arguments (but then kwargs would come through as a dictionary and be indistinguishable from one positional arg that happened to be a dict). The most general solution is to have several variants of gen.Callback depending on the expected signature.

Reply to this email directly or view it on GitHub:
#351 (comment)

@bdarnell
Copy link
Member

Returning (args, kwargs) is really awkward to work with. You don't want to have to do something like this on every call:

args, kwargs = yield gen.Task(asyncmongo...)
if kwargs.get('error'):
    # handle error
else:
    result = args[0]

You can kind of make an adapter with the Callback/Wait interface:

asyncmongo_function(callback=lambda result, error=None, cb=(yield gen.Callback('key')): cb((result, error)))
result, error = yield gen.Wait('key')

That could be made a little less clunky with support in Callback (and a few standard adapters could be predefined), but it's still messy (and making the analogous change to Task risks name collisions):

asyncmongo_function(callback=(yield gen.Callback('key', adapter=lambda result, error=None: (result, error)))

Maybe returning (args, kwargs) and requiring postprocessing is the best solution (especially since the postprocessing can easily be put into a function and not entangled with the gen framework).

def handle_mongo_result(result):
    args, kwargs = result
    if kwargs.get('error'): raise Exception(kwargs['error'])
    return args[0]
result = handle_mongo_result((yield gen.Task(...))

The case of a single positional argument is so common that it probably deserves to be handled specially, even if that means isinstance checks will be required for functions whose signatures vary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants