cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ilia Shakitko" <i.shaki...@tech.leaseweb.com>
Subject Re: Review Request 25435: [CLOUDSTACK-7159] Add "usageid" parameter to the "listUsageRecords" API call.
Date Mon, 08 Sep 2014 14:59:21 GMT


> On Sept. 8, 2014, 1:52 p.m., Rohit Yadav wrote:
> > api/src/org/apache/cloudstack/api/command/admin/usage/GetUsageRecordsCmd.java, lines
73-74
> > <https://reviews.apache.org/r/25435/diff/1/?file=682558#file682558line73>
> >
> >     Does this cause any issue? I would avoid this though there is no much different,
long will occupy some more bytes than Int, but it would require us to note change in the API.
> 
> Ilia Shakitko wrote:
>     1) UsageTypes are Inegers
>     2) usageRecord.getUsageType() is returning an Integer
>     3) public void setUsageType(Integer usageType) { ... }
>     
>     And If I'll change it to Long back, I'll have to either transform usageType (here:
"switch (usageType)") to an Integer (to support switch, as of usageTypes are Integers) or
use if-else statements, what I don't like in this particular place.
>     
>     Convinced? :)
> 
> Rohit Yadav wrote:
>     I'm just trying to help you with the review, my opinion is that one should avoid
changing API signature so either let Java auto-unbox/cast this from Long to Integer for you,
or fix usage types to Long.
> 
> Ilia Shakitko wrote:
>     If I will change UsageTypes to Long it will be more impact. But anyway, if it is
required I'll add a cast to Integer, which I don't like of course.

It is an integer everywhere, but in command parameters. I just wanted to make it more clear,
fixnig the type. But I can leave it as is.


- Ilia


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25435/#review52591
-----------------------------------------------------------


On Sept. 8, 2014, 1:45 p.m., Ilia Shakitko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25435/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2014, 1:45 p.m.)
> 
> 
> Review request for cloudstack, Alena Prokharchyk, Kishan Kavala, and Sheng Yang.
> 
> 
> Bugs: CLOUDSTACK-7159
>     https://issues.apache.org/jira/browse/CLOUDSTACK-7159
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Working with Usage server and usage records very often I need to get only records for
that particular usage ID. For example when filtering out network_bytes_received/sent with
big amount of data it's not very fast to process hundreds of objects looking for the only
one you need.
> It would be useful to have an ability to filter out usage records only for specific resource
ID.
> 
> This parch brings that to the API.
> 
> 
> Diffs
> -----
> 
>   api/src/org/apache/cloudstack/api/ApiConstants.java 6baa95c 
>   api/src/org/apache/cloudstack/api/command/admin/usage/GetUsageRecordsCmd.java 21a7e4a

>   server/src/com/cloud/usage/UsageServiceImpl.java d1f62aa 
> 
> Diff: https://reviews.apache.org/r/25435/diff/
> 
> 
> Testing
> -------
> 
> Tested cases:
> - usageid is specified w/o "type": an exception is thrown (correct)
> - provided usageid is not exists: an empty response is returned (since no records were
found, correct)
> - no usageid specified: work as is
> - an existing usageid specified (with type, for example type=4 or type=5): only records
for that usage type is returned
> 
> 
> Thanks,
> 
> Ilia Shakitko
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message