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

Error 500 when listing datasets with empty database #1363

Closed
wants to merge 1 commit into from
Closed

Error 500 when listing datasets with empty database #1363

wants to merge 1 commit into from

Conversation

rshk
Copy link
Contributor

@rshk rshk commented Dec 6, 2013

How to reproduce:

Complete error trace:

Error - <type 'exceptions.IndexError'>: list index out of range
URL: http://localhost:5000/api/3/action/package_list
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/weberror/errormiddleware.py', line 162 in __call__
  app_iter = self.application(environ, sr_checker)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/webob/dec.py', line 147 in __call__
  resp = self.call_func(req, *args, **self.kwargs)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/webob/dec.py', line 208 in call_func
  return self.func(req, *args, **kwargs)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/fanstatic/publisher.py', line 234 in __call__
  return request.get_response(self.app)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/webob/request.py', line 1053 in get_response
  application, catch_exc_info=False)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/webob/request.py', line 1022 in call_application
  app_iter = application(self.environ, start_response)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/webob/dec.py', line 147 in __call__
  resp = self.call_func(req, *args, **self.kwargs)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/webob/dec.py', line 208 in call_func
  return self.func(req, *args, **kwargs)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/fanstatic/injector.py', line 54 in __call__
  response = request.get_response(self.app)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/webob/request.py', line 1053 in get_response
  application, catch_exc_info=False)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/webob/request.py', line 1022 in call_application
  app_iter = application(self.environ, start_response)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/beaker/middleware.py', line 73 in __call__
  return self.app(environ, start_response)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/beaker/middleware.py', line 155 in __call__
  return self.wrap_app(environ, session_start_response)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/routes/middleware.py', line 131 in __call__
  response = self.app(environ, start_response)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/pylons/wsgiapp.py', line 125 in __call__
  response = self.dispatch(controller, environ, start_response)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/pylons/wsgiapp.py', line 324 in dispatch
  return controller(environ, start_response)
File '/tmp/tmpdJUWjF/venv/src/ckan/ckan/controllers/api.py', line 70 in __call__
  return base.BaseController.__call__(self, environ, start_response)
File '/tmp/tmpdJUWjF/venv/src/ckan/ckan/lib/base.py', line 346 in __call__
  res = WSGIController.__call__(self, environ, start_response)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/pylons/controllers/core.py', line 221 in __call__
  response = self._dispatch_call()
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/pylons/controllers/core.py', line 172 in _dispatch_call
  response = self._inspect_call(func)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/pylons/controllers/core.py', line 107 in _inspect_call
  result = self._perform_call(func, args)
File '/tmp/tmpdJUWjF/venv/local/lib/python2.7/site-packages/pylons/controllers/core.py', line 60 in _perform_call
  return func(**args)
File '/tmp/tmpdJUWjF/venv/src/ckan/ckan/controllers/api.py', line 189 in action
  result = function(context, request_data)
File '/tmp/tmpdJUWjF/venv/src/ckan/ckan/logic/__init__.py', line 419 in wrapped
  result = _action(context, data_dict, **kw)
File '/tmp/tmpdJUWjF/venv/src/ckan/ckan/logic/action/get.py', line 108 in package_list
  return list(zip(*query.execute())[0])
IndexError: list index out of range

This is clearly due to that zip() being performed on an empty list.
I'd replace that line with return [r[0] for r in query.execute()], which is way more readable too..

If you want a complete installation log: https://travis-ci.org/rshk/ckan-blackbox-testing/builds/15036108

(@rossjones, @seanh: first bug found thanks to the thing I was talking about yesterday..)

@wardi
Copy link
Contributor

wardi commented Dec 11, 2013

@rshk Good find. I used zip() there because it trimmed seconds off the response time for our server compared to the list comprehension. Would you modify your fix to catch the IndexError instead?

@ghost ghost assigned wardi Dec 12, 2013
@rshk
Copy link
Contributor Author

rshk commented Dec 12, 2013

This sounds very strange to me.. using zip() that way requires unnecessarily iterating not only over the whole result set, but over the fields in each single record too, in order to create a bunch of lists that would never be used...

How did you measure that using zip() is faster than list comprehension?

Would you modify your fix to catch the IndexError instead?

No, unless you prove me that there are actual advantages with that :)

And, anyways, I'd find some way to make that code more readable: using zip that way makes it harder to understand what it's going on exactly, while the list comprehension is much more explicit..

@wardi
Copy link
Contributor

wardi commented Dec 12, 2013

@rshk just ran my test again and you're right, the list comprehension is faster (and more readable).

This change looks good to me.

@seanh seanh closed this in 53a9ff2 Dec 12, 2013
@seanh
Copy link
Contributor

seanh commented Dec 12, 2013

I've merged this, even though github doesn't seem to believe it

@rshk
Copy link
Contributor Author

rshk commented Dec 13, 2013

I suspect github gets confused by the "fixes #1363" thing.. Ok, I'm not going to add "fix" comments to PRs anymore 😄

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

Successfully merging this pull request may close these issues.

None yet

3 participants