JoaoJandre commented on code in PR #13025:
URL: https://github.com/apache/cloudstack/pull/13025#discussion_r4405164762
##########
server/src/main/java/org/apache/cloudstack/storage/heuristics/HeuristicRuleHelper.java:
##########
@@ -248,28 +271,30 @@ protected Domain setDomainPresetVariable(long domainId) {
* in the code scope.
* <br>
* <br>
- * The JS script needs to return a valid UUID ({@link String}) of a
secondary storage, otherwise a {@link CloudRuntimeException} is thrown.
+ * The JS script needs to either return the valid UUID ({@link String}) of
a secondary storage or nothing. If a valid UUID is returned,
+ * this method returns the specific secondary storage; if nothing is
returned, this method returns null to allow allocation in any
+ * available secondary storage; otherwise a {@link CloudRuntimeException}
is thrown.
* @param rule the {@link String} representing the JS script.
* @param heuristicType used for building the preset variables accordingly
to the {@link HeuristicType} specified.
* @param obj can be from the following classes: {@link VMTemplateVO},
{@link SnapshotInfo} and {@link VolumeVO}.
* They are used to retrieve attributes for injecting in the JS
rule.
* @param zoneId used for injecting the {@link SecondaryStorage} preset
variables.
- * @return the {@link DataStore} returned by the script.
+ * @return the {@link DataStore} returned by the script, or null.
*/
public DataStore interpretHeuristicRule(String rule, HeuristicType
heuristicType, Object obj, long zoneId) {
try (JsInterpreter jsInterpreter = new
JsInterpreter(HEURISTICS_SCRIPT_TIMEOUT)) {
buildPresetVariables(jsInterpreter, heuristicType, zoneId, obj);
Object scriptReturn = jsInterpreter.executeScript(rule);
if (!(scriptReturn instanceof String)) {
- throw new CloudRuntimeException(String.format("Error while
interpreting heuristic rule [%s], the rule did not return a String.", rule));
+ logger.debug("Script did not return a string; allocating
resource in any available secondary storage.");
+ return null;
Review Comment:
I believe it would be good to add this into the description, unless I missed
it. It seems like an important change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
