I reviews all the related methods again, and the logic looks correct to
me. I added some additional comments and javadoc to clarify the
semantics of the collection interval parameter.
On 11/02/2010 09:59 AM, Ian Springer wrote:
Sure, will look into those changes. Thanks for reviewing.
On 11/02/2010 12:56 AM, Joseph Marques wrote:
> Ian, interesting fix.
>
> Looks like the enhancement for
> 'modifyDefaultCollectionIntervalForMeasurementDefinitions(4 args)' to
> support the amended JDoc was relatively straightforward. However, I
> think the logic:
>
> - boolean enableDisable = (collectionInterval> 0);
> + boolean enable = (collectionInterval>= 0);
>
> Should move into
> 'modifyDefaultCollectionIntervalForMeasurementDefinitions(5 args)' which
> implements the actual update logic. This will allow existing and future
> call paths to have the same post-condition.
>
> Looking deeper into
> 'modifyDefaultCollectionIntervalForMeasurementDefinitions(5 args)', I
> notice that we don't do any validation on the 'collectionInterval'
> parameter. When updating measurement schedules we make sure to call the
> 'verifyMinimumCollectionInterval' method, but we don't seem to prevent
> users from setting default collection intervals to invalid values.
>
> Can you add that validation?
>
> On 10/29/2010 07:44 PM, ips wrote:
> ...
>>
modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/measurement/MeasurementScheduleManagerBean.java
| 15 +-
>> 7 files changed, 143 insertions(+), 36 deletions(-)
>>
>> New commits:
>> commit 0a0aa07d0e4cdf71b1dee4a7631bd70e50715072
>> Author: Ian Springer<ian.springer(a)redhat.com>
>> Date: Fri Oct 29 19:43:38 2010 -0400
>>
>> add support to metric template view for specifying whether existing
schedules should be updated
> ...
>> diff --git
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/measurement/MeasurementScheduleManagerBean.java
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/measurement/MeasurementScheduleManagerBean.java
>> index ccd65bc..3dc8b05 100644
>> ---
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/measurement/MeasurementScheduleManagerBean.java
>> +++
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/measurement/MeasurementScheduleManagerBean.java
>> @@ -332,13 +332,16 @@ public class MeasurementScheduleManagerBean implements
MeasurementScheduleManage
>> }
> ...
>> + * @param collectionInterval if> 0, enable the metric with this
value as the the new collection
>> + * interval, in milliseconds; if == 0, enable
the metric with its current
>> + * collection interval; if< 0, disable
the metric
>> * @param updateExistingSchedules If true, then existing schedules for
this definition will also be updated.
>> */
>> @RequiredPermission(Permission.MANAGE_SETTINGS)
>> @@ -358,13 +361,13 @@ public class MeasurementScheduleManagerBean implements
MeasurementScheduleManage
>> return;
>> }
>>
>> - boolean enableDisable = (collectionInterval> 0);
>> + boolean enable = (collectionInterval>= 0);
>>
>> // batch the modifications to prevent the ORA error about IN clauses
containing more than 1000 items
>> for (int batchIndex = 0; (batchIndex<
measurementDefinitionIds.length); batchIndex += 1000) {
>> int[] batchIdArray =
ArrayUtils.copyOfRange(measurementDefinitionIds, batchIndex, batchIndex + 1000);
>>
>> - modifyDefaultCollectionIntervalForMeasurementDefinitions(subject,
batchIdArray, enableDisable,
>> + modifyDefaultCollectionIntervalForMeasurementDefinitions(subject,
batchIdArray, enable,
>> collectionInterval, updateExistingSchedules);
>> }
>> }
>>
>>
>> _______________________________________________
>> rhq-commits mailing list
>> rhq-commits(a)lists.fedorahosted.org
>>
https://fedorahosted.org/mailman/listinfo/rhq-commits
> _______________________________________________
> rhq-devel mailing list
> rhq-devel(a)lists.fedorahosted.org
>
https://fedorahosted.org/mailman/listinfo/rhq-devel
--
Ian Springer
Principal Software Developer
JBoss Operations Network
Red Hat
ian.springer(a)redhat.com