Ori Liel has posted comments on this change.

Change subject: restapi: Introducing Scheduling Policy API
......................................................................


Patch Set 1:

(7 comments)

I will probably have more comments, I just started looking

http://gerrit.ovirt.org/#/c/28093/1//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2014-05-25 20:51:53 +0300
Line 4: Commit:     Gilad Chaplik <gchap...@redhat.com>
Line 5: CommitDate: 2014-05-26 01:54:23 +0300
Line 6: 
Line 7: restapi: Introducing Scheduling Policy API
Please add a meaningful commit message, there's a lot to explain here
Line 8: 
Line 9: Closing the gap from oVirt 3.3.
Line 10: 
Line 11: Change-Id: I6c419f953a533e94d8be17cd9ba915ccd5a617a6


http://gerrit.ovirt.org/#/c/28093/1/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/LinkHelper.java
File 
backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/LinkHelper.java:

Line 374:         map = new ParentToCollectionMap(GlusterHookResource.class, 
GlusterHooksResource.class, Cluster.class);
Line 375:         TYPES.put(GlusterHook.class, map);
Line 376: 
Line 377:         map = new 
ParentToCollectionMap(SchedulingPolicyResource.class, 
SchedulingPoliciesResource.class, Cluster.class);
Line 378:         TYPES.put(SchedulingPolicy.class, map);
duplicate (see below)
Line 379: 
Line 380:         map = new ParentToCollectionMap(CapabiliyResource.class, 
CapabilitiesResource.class);
Line 381:         TYPES.put(VersionCaps.class, map);
Line 382: 


Line 410:         map = new 
ParentToCollectionMap(SchedulingPolicyUnitResource.class, 
SchedulingPolicyUnitsResource.class);
Line 411:         TYPES.put(SchedulingPolicyUnit.class, map);
Line 412: 
Line 413:         map = new 
ParentToCollectionMap(SchedulingPolicyResource.class, 
SchedulingPoliciesResource.class);
Line 414:         TYPES.put(SchedulingPolicy.class, map);
duplicate (see above)

TYPES.put(SchedulingPolicy.class, map), then
TYPES.put(SchedulingPolicy.class, map) again 

the second will override the first.
Line 415:     }
Line 416: 
Line 417:     /**
Line 418:      * Obtain the relative path to a top-level collection


http://gerrit.ovirt.org/#/c/28093/1/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
File 
backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd:

Line 1494:           <!-- deprecated - end-->
Line 1495:           <xs:element name="locked" type="xs:boolean" minOccurs="0" 
maxOccurs="1"/>
Line 1496:           <xs:element name="default_policy" type="xs:boolean" 
minOccurs="0" maxOccurs="1"/>
Line 1497:           <xs:element ref="filters" minOccurs="0" maxOccurs="1"/>
Line 1498:           <xs:element ref="weights" minOccurs="0" maxOccurs="1"/>
weights, filters, balances - these should all be external to SchedulingPolicy, 
i.e, have their own context: 

.../schecdulingpolicy/xxx/balances
.../schecdulingpolicy/xxx/weights
.../schecdulingpolicy/xxx/filters
Line 1499:           <xs:element ref="balances" minOccurs="0" maxOccurs="1"/>
Line 1500:           <xs:element ref="properties" minOccurs="0" maxOccurs="1"/>
Line 1501:         </xs:sequence>
Line 1502:       </xs:extension>


http://gerrit.ovirt.org/#/c/28093/1/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/SchedulingPolicyMapper.java
File 
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/SchedulingPolicyMapper.java:

Line 85:         }
Line 86:         if (model.isSetDefaultPolicy()) {
Line 87:             entity.setDefaultPolicy(model.isDefaultPolicy());
Line 88:         }
Line 89:         if (model.isSetFilters() && model.getFilters().isSetFilters()) 
{
filters should be a separate business-entity and have their own mapper. They 
will not exist as a reference inside SchedulingPolicy
Line 90:             entity.setFilters(new ArrayList<Guid>());
Line 91:             entity.setFilterPositionMap(new HashMap<Guid, Integer>());
Line 92:             for (Filter filter : model.getFilters().getFilters()) {
Line 93:                 if (filter.isSetSchedulingPolicyUnit() && 
filter.getSchedulingPolicyUnit().isSetId()) {


Line 98:                 }
Line 99:             }
Line 100:         }
Line 101:         if (model.isSetWeights() && 
model.getWeights().isSetWeights()) {
Line 102:             entity.setFunctions(new ArrayList<Pair<Guid, Integer>>());
weights should be a separate business-entity and have their own mapper. They 
will not exist as a reference inside SchedulingPolicy
Line 103:             for (Weight Weight : model.getWeights().getWeights()) {
Line 104:                 if (Weight.isSetSchedulingPolicyUnit() && 
Weight.getSchedulingPolicyUnit().isSetId()) {
Line 105:                     Guid WeightGuid = 
GuidUtils.asGuid(Weight.getSchedulingPolicyUnit().getId());
Line 106:                     entity.getFunctions().add(new Pair<Guid, 
Integer>(WeightGuid,


Line 107:                             Weight.isSetFactor() ? Weight.getFactor() 
: 1));
Line 108:                 }
Line 109:             }
Line 110:         }
Line 111:         if (model.isSetBalances() && 
model.getBalances().isSetBalances()
balances should be a separate business-entity and have their own mapper. They 
will not exist as a reference inside SchedulingPolicy
Line 112:                 && 
model.getBalances().getBalances().isSetSchedulingPolicyUnit()
Line 113:                 && 
model.getBalances().getBalances().getSchedulingPolicyUnit().isSetId()) {
Line 114:             
entity.setBalance(GuidUtils.asGuid(model.getBalances().getBalances().getSchedulingPolicyUnit().getId()));
Line 115:         }


-- 
To view, visit http://gerrit.ovirt.org/28093
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6c419f953a533e94d8be17cd9ba915ccd5a617a6
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Artyom Lukianov <aluki...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Martin Sivák <msi...@redhat.com>
Gerrit-Reviewer: Ori Liel <ol...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to