GutoVeronezi commented on code in PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#discussion_r2618995375
##########
api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotFromVMSnapshotCmd.java:
##########
@@ -165,7 +165,7 @@ public void create() throws ResourceAllocationException {
@Override
public void execute() {
VMSnapshot vmSnapshot =
_vmSnapshotService.getVMSnapshotById(getVMSnapshotId());
- logger.info("CreateSnapshotFromVMSnapshotCmd with vm snapshot {} with
id {} and snapshot [id: {}, uuid: {}]", vmSnapshot, getVMSnapshotId(),
getEntityId(), getEntityUuid());
+ logger.info("CreateSnapshotFromVMSnapshotCmd with {} and snapshot [id:
{}, uuid: {}]", vmSnapshot, getEntityId(), getEntityUuid());
Review Comment:
I left only the snapshot object to be logged because its `toString()`
already prints the necessary data; soon I'll post the tests.
##########
api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateEgressFirewallRuleCmd.java:
##########
@@ -255,11 +255,8 @@ public void create() {
}
} catch (NetworkRuleConflictException ex) {
String message = "Network rule conflict: ";
- if (!logger.isTraceEnabled()) {
- logger.info(message + ex.getMessage());
- } else {
- logger.trace(message, ex);
- }
+ logger.error( "Network rule conflict: {}", ex.getMessage());
Review Comment:
```suggestion
logger.error("{}{}", message, ex.getMessage());
```
##########
api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java:
##########
@@ -540,19 +533,15 @@ public List<OVFNetworkTO>
getNetPrerequisitesFromDocument(Document doc) throws I
private void matchNicsToNets(Map<String, OVFNetworkTO> nets, Node
systemElement) {
final DocumentTraversal traversal = (DocumentTraversal) systemElement;
final NodeIterator iterator =
traversal.createNodeIterator(systemElement, NodeFilter.SHOW_ELEMENT, null,
true);
- if (logger.isTraceEnabled()) {
- logger.trace(String.format("starting out with %d
network-prerequisites, parsing hardware",nets.size()));
- }
+ logger.trace("Starting out with {} network-prerequisites, parsing
hardware", nets.size());
int nicCount = 0;
for (Node n = iterator.nextNode(); n != null; n = iterator.nextNode())
{
final Element e = (Element) n;
if ("rasd:Connection".equals(e.getTagName())) {
nicCount++;
String name = e.getTextContent(); // should be in our nets
if(nets.get(name) == null) {
- if(logger.isInfoEnabled()) {
- logger.info(String.format("found a nic definition
without a network definition byname %s, adding it to the list.", name));
- }
+ logger.info("Found a nic definition without a network
definition byname {}, adding it to the list.", name);
Review Comment:
```suggestion
logger.info("Found a nic definition without a network
definition by name {}, adding it to the list.", name);
```
##########
api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkACLListCmd.java:
##########
@@ -128,7 +128,7 @@ public long getEntityOwnerId() {
} else {
account = CallContext.current().getCallingAccount();
if (!Account.Type.ADMIN.equals(account.getType())) {
- logger.warn(String.format("Only Root Admin can create global
ACLs. Account [%s] cannot create any global ACL.", account));
+ logger.error("Only Root Admin can create global ACLs. {}
cannot create any global ACL.", account);
Review Comment:
I left only the account object to be logged because its `toString()` already
prints the necessary data; soon I'll post the tests.
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/host/UpdateHostCmd.java:
##########
@@ -147,7 +147,7 @@ public void execute() {
this.setResponseObject(hostResponse);
} catch (Exception e) {
Host host = _entityMgr.findById(Host.class, getId());
- logger.debug("Failed to update host: {} with id {}", host,
getId(), e);
+ logger.debug("Failed to update {}", host, e);
Review Comment:
I left only the host object to be logged because its `toString()` already
prints the necessary data; soon I'll post the tests.
##########
api/src/main/java/org/apache/cloudstack/context/LogContext.java:
##########
@@ -207,9 +205,7 @@ public static void unregister() {
LogContext context = s_currentContext.get();
if (context != null) {
s_currentContext.remove();
- if (LOGGER.isTraceEnabled()) {
- LOGGER.trace("Unregistered: " + context);
- }
+ LOGGER.trace("Unregistered: {}", () -> context);
Review Comment:
```suggestion
LOGGER.trace("Unregistered: {}", context);
```
--
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]
