Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDK-320: Replace System.out.println with logger #252

Merged
merged 6 commits into from
Sep 10, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import org.apache.commons.lang3.StringUtils;
import org.apache.maven.plugin.AbstractMojo;
import org.apache.maven.plugin.MojoExecutionException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.net.URI;
Expand All @@ -25,6 +27,7 @@ abstract class AbstractDockerMojo extends AbstractMojo {
protected static final String MYSQL_5_6 = "mysql:5.6";
protected static final String DEFAULT_MYSQL_DB_URI = "jdbc:mysql://localhost:" + DEFAULT_MYSQL_EXPOSED_PORT + "/";
protected static final String API_VERSION = "1.18";
private static final Logger logger = LoggerFactory.getLogger(AbstractDockerMojo.class);

private static final String NOT_LINUX_UNABLE_TO_CONNECT_MESSAGE = "\n\n\nCould not connect to Docker at " +
"%s\n\n Please make sure Docker is running.\n\n If you are using 'Docker Toolbox', " +
Expand Down Expand Up @@ -111,7 +114,7 @@ protected Container findContainer(String id){
}

protected void showMessage(String message) {
System.out.println("\n" + message);
logger.info("\n{}", message);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import org.openmrs.maven.plugins.model.Artifact;
import org.openmrs.maven.plugins.model.NodeDistro;
import org.openmrs.maven.plugins.model.PackageJson;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.twdata.maven.mojoexecutor.MojoExecutor;

import java.io.File;
Expand Down Expand Up @@ -63,6 +65,8 @@ public class OwaHelper {

private Wizard wizard;

private static final Logger logger = LoggerFactory.getLogger(OwaHelper.class);

//enable chaining
public OwaHelper setInstallationDir(File installationDir) {
this.installationDir = installationDir;
Expand Down Expand Up @@ -300,7 +304,7 @@ private String runProcessAndGetFirstResponseLine(String command, @Nullable Strin
lines = IOUtils.readLines(process.getInputStream(), StandardCharsets.UTF_8);
process.waitFor();
} catch (InterruptedException | IOException e) {
System.out.println(e.getMessage());
logger.error("Exception: ", e);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the first parameter be the exception message?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dkayiwa I changed this

}

if (process != null && process.exitValue() == 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

public class ServerHelper {
private final Wizard wizard;
private final Logger log = LoggerFactory.getLogger(ServerHelper.class);
private static final Logger log = LoggerFactory.getLogger(ServerHelper.class);

private static final int MAX_USHORT_VALUE = 65535;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import org.apache.maven.model.io.xpp3.MavenXpp3Writer;
import org.apache.maven.plugin.MojoExecutionException;
import org.openmrs.maven.plugins.utility.SDKConstants;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.File;
import java.io.FileOutputStream;
Expand Down Expand Up @@ -54,7 +56,7 @@ public class Server extends BaseSdkProperties {
private static final String OLD_PROPERTIES_FILENAME = "backup.properties";
public static final String OWA_DIRECTORY = "owa";

private static final String CANNOT_CREATE_LINK_MSG = "\nCannot create a link at %s due to:\n%s\n" +
private static final String CANNOT_CREATE_LINK_MSG = "\nCannot create a link at {} due to:\n{}\n" +
"The project will be built in random order.\n" +
"Please try running the command as an administrator.\n";

Expand All @@ -76,6 +78,8 @@ public class Server extends BaseSdkProperties {

private boolean interactiveMode;

private static final Logger logger = LoggerFactory.getLogger(Server.class);

public static class ServerBuilder {
private final Server server = new Server();
public ServerBuilder(Server server){
Expand Down Expand Up @@ -333,18 +337,17 @@ private boolean linkProject(Project project) {
if (Files.isSymbolicLink(linkPath)) {
try {
if (!Files.isSameFile(Paths.get(project.getPath()), linkPath)) {
System.out.println("\nDeleting a link at " + link.getAbsolutePath() + " as it points to a different location.");
logger.info("\nDeleting a link at {} as it points to a different location.", link.getAbsolutePath());
Files.delete(linkPath);
} else {
return true;
}
} catch (IOException e) {
System.out.printf((CANNOT_CREATE_LINK_MSG) + "%n", link.getAbsolutePath(), e.getMessage());
logger.error(CANNOT_CREATE_LINK_MSG, link.getAbsolutePath(), e.getMessage());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is usually great to pass the exception object at the end

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the changes

return false;
}
} else {
System.out.printf((CANNOT_CREATE_LINK_MSG) + "%n", link.getAbsolutePath(),
"The file or directory already exists!\nPlease delete it manually and try again.");
logger.error(CANNOT_CREATE_LINK_MSG, link.getAbsolutePath(), "The file or directory already exists!\nPlease delete it manually and try again.");
return false;
}
}
Expand All @@ -353,7 +356,7 @@ private boolean linkProject(Project project) {
try {
Files.createSymbolicLink(linkPath, Paths.get(project.getPath()));
} catch (IOException e) {
System.out.printf((CANNOT_CREATE_LINK_MSG) + "%n", link.getAbsolutePath(), e.getMessage());
logger.error(CANNOT_CREATE_LINK_MSG, link.getAbsolutePath(), e.getMessage());
return false;
}
}
Expand Down Expand Up @@ -456,7 +459,7 @@ private void unlinkProject(Project project) {
try {
Files.deleteIfExists(Paths.get(link.getAbsolutePath()));
} catch (IOException e) {
System.out.println("\nCould not delete link at " + link.getAbsolutePath());
logger.error("\nCould not delete link at {}", link.getAbsolutePath());
}
}

Expand Down Expand Up @@ -868,7 +871,7 @@ public void deleteServerTmpDirectory() {
try {
FileUtils.deleteDirectory(tmpDirectory);
} catch (IOException e) {
System.out.println("Could not delete tmp directory");
logger.error("Could not delete tmp directory");
}
}
}
Expand Down
Loading