DaanHoogland commented on code in PR #12419:
URL: https://github.com/apache/cloudstack/pull/12419#discussion_r2693622275
##########
engine/schema/src/main/java/com/cloud/upgrade/dao/DatabaseAccessObject.java:
##########
@@ -98,7 +98,7 @@ public boolean indexExists(Connection conn, String tableName,
String indexName)
return true;
}
} catch (SQLException e) {
- logger.debug(String.format("Index %s doesn't exist, ignoring
exception:", indexName, e.getMessage()));
+ logger.debug(String.format("Index %s doesn't exist, ignoring
exception:", indexName), e.getMessage());
Review Comment:
```suggestion
logger.debug("Index {} doesn't exist, ignoring exception: {}",
indexName, e.getMessage());
```
##########
plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java:
##########
@@ -181,10 +181,9 @@ public Pair<Boolean, Long> getCommandHostDelegation(long
hostId, Command cmd) {
logger.debug("We are returning the default host to execute
commands because the target hypervisor of the source data is not XenServer.");
return defaultHostToExecuteCommands;
}
- // only now can we decide, now we now we're only deciding for ourselves
if (cmd instanceof StorageSubSystemCommand) {
if (logger.isTraceEnabled()) {
- logger.trace(String.format("XenServer StrorageSubSystemCommand
re always executed in sequence (command of type %s to host %l).",
cmd.getClass(), hostId));
+ logger.trace(String.format("XenServer StrorageSubSystemCommand
is always executed in sequence (command of type %s to host %s).",
cmd.getClass(), hostId));
}
Review Comment:
```suggestion
logger.trace("XenServer StrorageSubSystemCommand is always
executed in sequence (command of type {} to host {}).", cmd.getClass(), hostId);
```
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java:
##########
@@ -83,7 +83,7 @@ public Boolean getAllowUserDrivenBackups() {
public void execute() {
try {
if (StringUtils.isAllEmpty(getName(), getDescription()) &&
getAllowUserDrivenBackups() == null) {
- throw new InvalidParameterValueException(String.format("Can't
update Backup Offering [id: %s] because there are no parameters to be updated,
at least one of the",
+ throw new InvalidParameterValueException(String.format("Can't
update Backup Offering [id: %s] because there are no parameters to be updated,
at least one of the " +
"following should be informed: name, description or
allowUserDrivenBackups.", id));
Review Comment:
```suggestion
throw new InvalidParameterValueException(String.format(
"Can't update Backup Offering [id: %s] because there are
no parameters to be updated,” +
" at least one of the following should be informed:
name, description or allowUserDrivenBackups.",
id));
```
(just readability??)
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java:
##########
@@ -497,6 +498,11 @@ public void process(final Answer[] answers) {
*/
protected abstract boolean isClosed();
+ @Override
+ public int hashCode() {
+ return Objects.hash(_id, _uuid, _name);
Review Comment:
or add _uuid and _name comparison to `equals()`? actually only _uuid and
_name makes more sense.
##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -1730,7 +1730,7 @@ public void stop(final String vmUuid) throws
ResourceUnavailableException {
} catch (final OperationTimedoutException e) {
throw new AgentUnavailableException(String.format("Unable to stop
vm [%s] because the operation to stop timed out", vmUuid), e.getAgentId(), e);
} catch (final ConcurrentOperationException e) {
- throw new CloudRuntimeException(String.format("Unable to stop vm
because of a concurrent operation", vmUuid), e);
+ throw new CloudRuntimeException(String.format("Unable to stop vm:
%s because of a concurrent operation", vmUuid), e);
Review Comment:
```suggestion
throw new CloudRuntimeException(String.format("Unable to stop vm
[%s] because of a concurrent operation", vmUuid), e);
```
##########
plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java:
##########
@@ -240,12 +241,16 @@ public void tearDown() throws Exception {
}
//@Test(expected = InvalidParameterValueException.class)
+ @Test
Review Comment:
true for the below as well. or-else we could either fix or clean up this
test file and keep only sensible tests.
##########
server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java:
##########
@@ -1041,7 +1041,7 @@ private void processWork(final HaWorkVO work) {
try {
if (vm != null && !VmHaEnabled.valueIn(vm.getDataCenterId())) {
if (logger.isDebugEnabled()) {
- logger.debug(String.format("VM high availability manager
is disabled, rescheduling the HA work %s, for the VM %s (id) to retry later in
case VM high availability manager is enabled on retry attempt", work,
vm.getName(), vm.getId()));
+ logger.debug("VM high availability manager is disabled,
rescheduling the HA work {}, for the VM {} (id: {}) to retry later in case VM
high availability manager is enabled on retry attempt", work, vm.getName(),
vm.getId());
}
long nextTime = getRescheduleTime(wt);
Review Comment:
```suggestion
logger.debug("VM high availability manager is disabled,
rescheduling the HA work {}, for the VM {} (id: {}) to retry later in case VM
high availability manager is enabled on retry attempt", work, vm.getName(),
vm.getId());
long nextTime = getRescheduleTime(wt);
```
##########
plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java:
##########
@@ -34,6 +34,7 @@ public class ADLdapUserManagerImpl extends
OpenLdapUserManagerImpl implements Ld
private static final String MICROSOFT_AD_MEMBERS_FILTER = "memberOf";
@Override
+ @SuppressWarnings("BanJNDI")
Review Comment:
we should probably take this one serious. are we blocking jndi somehow?
##########
.github/linters/codespell.txt:
##########
@@ -188,6 +188,7 @@ environmnet
equivalant
erro
erronous
+errorprone
Review Comment:
can we try this? it makes sense.
--
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]
