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 :)
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) volumes( context, 1, size) < 1:
380 + if quota.allowed_
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