Code review comment for lp:~khussein/swift/authn

Revision history for this message
Mike Barton (redbo) wrote :

> self.response_status = status

self.response_status is storing per-request info in a shared object. That could be wrong under concurrency.

> ''.join(response_itr)
> return [resp.read()]
> response.content_length = sum(map(len, response.app_iter))

Storing many GB of data in RAM is a bad idea.

> usersConfig.readfp(open('/etc/openstack/users.ini'))

You shouldn't re-read and parse the basic auth credentials file on every request. The local imports there are kind of ugly too.

> def validateCreds(self, username, password):

only major pep8 violation I see is those camel caps.

> - 'auth=swift.auth.server:app_factory',
> + #'auth=swift.auth.server:app_factory',
> + 'auth=swift.auth.basicauth:app_factory',

Was leaving it this way a mistake? The basic auth isn't super useful right now, and removing the devauth filter completely breaks our dev and testing environments.

review: Needs Fixing

« Back to merge proposal