Code review comment for lp:~vishvananda/nova/quotas

Hi Vish!

Nice stuff!

Some small stuff I noticed...

87 + project_quota = quota._get_quota(None, project_id)

You might as well make quota._get_quota() into a public function since it's used outside of the quote.py file, so a rename to quota.get_quota() would be good (also a docstring on that function would be super-cool that indicates the function returns a set of default quota configuration options, overridden with any non-null value stored in the db for that project...)

379 + size = int(size)
380 + if quota.allowed_volumes(context, 1, size) < 1:
381 + logging.warn("Quota exceeeded for %s, tried to create %sG volume",
382 + context.project.id, size)
383 + raise QuotaError("Volume quota exceeded. You cannot "
384 + "create a volume of size %s" %
385 + size)

The above interface for determining whether a quote has been exceeded could be refactored slightly to reduce misuse. Instead of having the caller remember to cast size to an int, the quota.allowed_volumes() would be a better place to do that cast since it means one less assumption on the part of the interface.

Also, you may consider changing the interface to return a boolean instead of a number of allowed volumes. This may improve readability.

For example, you could have the following code instead:

if quota.volumes_exceeded(context, size):
    ... raise error, log, etc..

Hope that makes sense...I think that would make the interface a bit more intuitive. You could even keep the quota.allowed_volumes() function and make a small convenience wrapper around it to remove some of the awkwardness in the current interface.

Anyway, just some thoughts. Great work overall, Vish :)

-jay