Mike Kolesnik has posted comments on this change.
Change subject: engine: Add conditional execution to host deploy
......................................................................
Patch Set 2: (4 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsDeploy.java
Line 217
Line 218
Line 219
Line 220
Line 221
This might be available in Java 8 if they release closures support.
Currently I can think of another way to simplify this code, using methods for
common usage, such as:
private final Callable<?>[] _customizationDialog = new Callable<?>[] {
createEnvironmentSetter(
key,
value),
createXYZEnvironmentSetter(
key,
value)
}
And we define these methods:
private Callable<?> createEnvironmentSetter(String key, Object value) {
return new Callable<Object>() { public Object call() throws Exception {
_parser.cliEnvironmentSet(
key,
value
);
return null;
}};
}
private Callable<?> createXYZEnvironmentSetter(String key, Object value) {
return new Callable<Object>() { @CallWhen(XYZ.class) public Object call()
throws Exception {
_parser.cliEnvironmentSet(
key,
value
);
return null;
}};
}
Annotations by definition have to get only static values so we cant pass
variables to them.
Additionally annotations in Java 7 are very limited and can't be set on an
anonymous class (IIRC in Java 8 it should be possible), otherwise I'd initially
put the annotation on a class level making it much more simple and elegant.
Alternatively, we can not use annotation and have the code in the method, so
the createEnvironmentSetter method would look like this:
private Callable<?> createEnvironmentSetter(String key, Object value,
CustomizationCondition ... conditions) {
return new Callable<Object>() { public Object call() throws Exception {
if (_customizationConditions.containsAll(Arrays.asList(conditions)) {
_parser.cliEnvironmentSet(
key,
value
);
}
else {
_parse.cliNoop();
}
return null;
}};
}
Which I think is not such a bad design, but it limits the conditions only to
env setters (which is where its currently used anyway). Whoever wants to use
conditions in the future would need to have this code in his callable.
Please let me know what you think.
Line 225: private @interface CallWhen {
Line 226: /**
Line 227: * @return A condition that determines if the customization
should run.
Line 228: */
Line 229: CustomizationCondition value();
Done
Line 230: }
Line 231: /**
Line 232: * A set of conditions under which the conditional customizations
should run.
Line 233: */
Line 302: }},
Line 303: new Callable<Object>() { public Object call() throws
Exception {
Line 304: _parser.cliEnvironmentSet(
Line 305: NetEnv.IPTABLES_ENABLE,
Line 306:
_customizationConditions.contains(CustomizationCondition.IPTABLES_OVERRIDE)
I'm not sure, can we skip sending it in case it's not supposed to happen, i.e.
the default is false?
Currently changing but please let me know
Line 307: );
Line 308: return null;
Line 309: }},
Line 310: new Callable<Object>()
{@CallWhen(CustomizationCondition.IPTABLES_OVERRIDE)
Line 412
Line 413
Line 414
Line 415
Line 416
After reviewing the termination dialog when doing this change, I didn't see
such a use case there.
Either way, I think it can be done in a separate patch if we do find such a use
case.
--
To view, visit http://gerrit.ovirt.org/16849
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I98b4fe7d11cdb267664113ac7312a0cd12268f94
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
