Permalink
Browse files

SLF4J-related fixes (#569)

* Fix misspellings of "intended"

* Utilize parameterized logging and remove calls to toString()
** As suggested in the SLF4J FAQ: https://www.slf4j.org/faq.html#logging_performance
parameterized logging can improve the efficiency of logging calls when
logging at the specified level is disabled.

In addition, per the FAQ: https://www.slf4j.org/faq.html#string_contents
toString() is called automatically.

These changes were suggested and automatically made by SLF4J Helper:
http://plugins.netbeans.org/plugin/72557/

* Also log the IOException
** SLF4J has special support for the last argument to a logger call being
an instance of Throwable: https://www.slf4j.org/faq.html#string_or_object

* Fix two SLF4J logger calls
** There were an inconsistent number of formatting anchors ('{}') which
can cause information not to be logged as expected.

These issues were identified by SLF4J Helper for NetBeans:
http://plugins.netbeans.org/plugin/72557/
  • Loading branch information...
Daniel Trebbien authored and tgianos committed Jul 24, 2017
1 parent 68775d3 commit cdba9eb312ddfb2230b9abfcd10cff8c15fe9ac2
@@ -197,7 +197,7 @@ private boolean canExecute(final String runScriptFile) {
// Helper method to add write permissions to a directory for the group owner
private void makeDirGroupWritable(final String dir) throws GenieServerException {
log.debug("Adding write permissions for the directory " + dir + " for the group.");
log.debug("Adding write permissions for the directory {} for the group.", dir);
final CommandLine commandLIne = new CommandLine("sudo").addArgument("chmod").addArgument("g+w")
.addArgument(dir);
@@ -238,10 +238,10 @@ protected synchronized void createUser(final String user, final String group) th
// We create the group and ignore the error as it will fail if group already exists.
// If the failure is due to some other reason, then user creation will fail and we catch that.
try {
log.debug("Running command to create group: [" + groupCreateCommandLine.toString() + "]");
log.debug("Running command to create group: [{}]", groupCreateCommandLine);
this.executor.execute(groupCreateCommandLine);
} catch (IOException ioexception) {
log.debug("Group creation threw an error as it might already exist");
log.debug("Group creation threw an error as it might already exist", ioexception);
}
}
@@ -252,7 +252,7 @@ protected synchronized void createUser(final String user, final String group) th
userCreateCommandLine.addArgument("-M");
try {
log.debug("Running command to create user: [" + userCreateCommandLine.toString() + "]");
log.debug("Running command to create user: [{}]", userCreateCommandLine);
this.executor.execute(userCreateCommandLine);
} catch (IOException ioexception) {
throw new GenieServerException("Could not create user " + user + " with exception " + ioexception);
@@ -99,7 +99,7 @@ public String createApplication(
@Valid
final Application app
) throws GenieException {
log.debug("Called with application: {}", app.toString());
log.debug("Called with application: {}", app);
final Optional<String> appId = app.getId();
if (appId.isPresent() && this.applicationRepo.exists(appId.get())) {
throw new GenieConflictException("An application with id " + appId.get() + " already exists");
@@ -165,7 +165,7 @@ public void updateApplication(
throw new GenieBadRequestException("Application id inconsistent with id passed in.");
}
log.debug("Called with app {}", updateApp.toString());
log.debug("Called with app {}", updateApp);
this.updateAndSaveApplicationEntity(this.findApplication(id), updateApp);
}
@@ -224,7 +224,7 @@ public void deleteCommand(
@NotBlank(message = "No id entered. Unable to delete.")
final String id
) throws GenieException {
log.debug("Called to delete command config with id {}");
log.debug("Called to delete command config with id {}", id);
final CommandEntity commandEntity = this.findCommand(id);
//Remove the command from the associated Application references
@@ -63,7 +63,7 @@ public void save(
final File attachment = new File(attachmentDirectory, jobId + "/" + filename);
try {
FileUtils.copyInputStreamToFile(content, attachment);
log.info("Saved " + filename + " to " + attachment.getAbsolutePath());
log.info("Saved {} to {}", filename, attachment.getAbsolutePath());
} catch (final IOException ioe) {
throw new GenieServerException(ioe);
}
@@ -18,7 +18,7 @@
package com.netflix.genie.test.categories;
/**
* Interface intented to be used as a JUnit category to flag tests as generating documentation.
* Interface intended to be used as a JUnit category to flag tests as generating documentation.
*
* @author tgianos
* @since 3.0.0
@@ -18,7 +18,7 @@
package com.netflix.genie.test.categories;
/**
* Interface intented to be used as a JUnit category to flag tests as integration tests.
* Interface intended to be used as a JUnit category to flag tests as integration tests.
*
* @author tgianos
* @since 3.0.0
@@ -18,7 +18,7 @@
package com.netflix.genie.test.categories;
/**
* Interface intented to be used as a JUnit category to flag tests as unit tests.
* Interface intended to be used as a JUnit category to flag tests as unit tests.
*
* @author tgianos
* @since 3.0.0
@@ -66,7 +66,7 @@ public AuthenticationManager authenticationManagerBean() throws Exception {
protected void configureGlobal(final AuthenticationManagerBuilder auth) throws Exception {
if (this.providers != null) {
for (final AuthenticationProvider provider : this.providers) {
log.debug("Adding authentication provider {} to authentication provider.", provider.toString());
log.debug("Adding authentication provider {} to authentication provider.", provider);
auth.authenticationProvider(provider);
}
} else {
@@ -172,7 +172,7 @@ public Cluster selectCluster(
final Map<String, String> tags = Maps.newHashMap();
try {
if (this.isConfigured.get() && this.script != null && this.script.get() != null) {
log.debug("Evaluating script for job " + jobRequest.getId().orElse("without id"));
log.debug("Evaluating script for job {}", jobRequest.getId().orElse("without id"));
final Bindings bindings = new SimpleBindings();
bindings.put(CLUSTERS_BINDING, this.mapper.writeValueAsString(clusters));
bindings.put(JOB_REQUEST_BINDING, this.mapper.writeValueAsString(jobRequest));
@@ -191,7 +191,7 @@ public Cluster selectCluster(
}
}
}
log.warn("Script returned a cluster not in the input list: " + clusterId);
log.warn("Script returned a cluster not in the input list: {}", clusterId);
} else {
log.debug("Script returned null");
tags.put(STATUS_TAG_KEY, STATUS_TAG_NOT_CONFIGURED);
@@ -204,7 +204,7 @@ public Cluster selectCluster(
} catch (final Exception e) {
tags.put(STATUS_TAG_KEY, STATUS_TAG_FAILED);
tags.put(EXCEPTION_TAG_KEY, e.getClass().getName());
log.error("Unable to execute script due to", e.getMessage(), e);
log.error("Unable to execute script due to {}", e.getMessage(), e);
return null;
} finally {
this.registry
@@ -294,7 +294,7 @@ public void refresh() {
final InputStream fis = Files.newInputStream(scriptDestinationPath);
final InputStreamReader reader = new InputStreamReader(fis, UTF_8)
) {
log.debug("Compiling " + scriptFileSource);
log.debug("Compiling {}", scriptFileSource);
this.script.set(compilable.compile(reader));
}
@@ -287,7 +287,7 @@ private void cleanupProcesses(final String jobId) {
});
}
} catch (final GenieException ge) {
log.error("Unable to cleanup process for job due to exception. " + jobId, ge);
log.error("Unable to cleanup process for job due to exception. {}", jobId, ge);
this.processGroupCleanupFailureRate.increment();
}
}
@@ -463,7 +463,7 @@ private void deleteDependenciesDirectory(final File dependencyDirectory) throws
deleteCommand.addArgument("rm");
deleteCommand.addArgument("-rf");
deleteCommand.addArgument(dependencyDirectory.getCanonicalPath());
log.debug("Delete command is {}", deleteCommand.toString());
log.debug("Delete command is {}", deleteCommand);
this.executor.execute(deleteCommand);
} else {
FileUtils.deleteDirectory(dependencyDirectory);
@@ -514,7 +514,7 @@ private boolean processJobDir(final Job job) throws GenieException, IOException
this.executor.setWorkingDirectory(jobDir);
log.debug("Archive command : {}", commandLine.toString());
log.debug("Archive command : {}", commandLine);
this.executor.execute(commandLine);
// Upload the tar file to remote location
@@ -532,7 +532,7 @@ private boolean processJobDir(final Job job) throws GenieException, IOException
deleteCommand.addArgument(localArchiveFile.getCanonicalPath());
this.executor.setWorkingDirectory(jobDir);
log.debug("Delete command: {}", deleteCommand.toString());
log.debug("Delete command: {}", deleteCommand);
this.executor.execute(deleteCommand);
} else if (!localArchiveFile.delete()) {
log.error("Failed to delete archive file for job: {}", jobId);
@@ -136,7 +136,7 @@ public synchronized void onLeaderEvent(final AbstractLeaderEvent leaderEvent) {
private void cancelTasks() {
for (final ScheduledFuture<?> future : this.futures) {
log.info("Attempting to cancel thread {}", future.toString());
log.info("Attempting to cancel thread {}", future);
if (future.cancel(true)) {
log.info("Successfully cancelled.");
} else {
@@ -62,7 +62,7 @@ public LocalLeader(final ApplicationEventPublisher publisher, final boolean isLe
@EventListener
public void startLeadership(final ContextRefreshedEvent event) {
if (this.isLeader) {
log.debug("Starting Leadership due to " + event);
log.debug("Starting Leadership due to {}", event);
this.publisher.publishEvent(new OnGrantedEvent(this, null, "leader"));
}
}
@@ -75,7 +75,7 @@ public void startLeadership(final ContextRefreshedEvent event) {
@EventListener
public void stopLeadership(final ContextClosedEvent event) {
if (this.isLeader) {
log.debug("Stopping Leadership due to " + event);
log.debug("Stopping Leadership due to {}", event);
this.publisher.publishEvent(new OnRevokedEvent(this, null, "leader"));
}
}

0 comments on commit cdba9eb

Please sign in to comment.