Skip to content

Commit

Permalink
FindBugs fixups, mostly related to possible toString() NPEs
Browse files Browse the repository at this point in the history
  • Loading branch information
Steven Schlansker committed Jul 7, 2015
1 parent 919b23f commit cf057c0
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 18 deletions.
Expand Up @@ -3,6 +3,7 @@
import java.io.IOException; import java.io.IOException;
import java.nio.file.Path; import java.nio.file.Path;
import java.util.List; import java.util.List;
import java.util.Objects;


import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectMapper;
Expand Down Expand Up @@ -151,7 +152,7 @@ private void downloadFilesFromLocalDownloadService(List<S3Artifact> s3Artifacts,
private void downloadRemoteArtifact(RemoteArtifact remoteArtifact, ArtifactManager artifactManager, SingularityExecutorTask task) { private void downloadRemoteArtifact(RemoteArtifact remoteArtifact, ArtifactManager artifactManager, SingularityExecutorTask task) {
Path fetched = artifactManager.fetch(remoteArtifact); Path fetched = artifactManager.fetch(remoteArtifact);


if (fetched.getFileName().toString().endsWith(".tar.gz")) { if (Objects.toString(fetched.getFileName()).endsWith(".tar.gz")) {
artifactManager.untar(fetched, task.getTaskDefinition().getTaskDirectoryPath()); artifactManager.untar(fetched, task.getTaskDefinition().getTaskDirectoryPath());
} else { } else {
artifactManager.copy(fetched, task.getTaskDefinition().getTaskAppDirectoryPath()); artifactManager.copy(fetched, task.getTaskDefinition().getTaskAppDirectoryPath());
Expand Down
Expand Up @@ -7,8 +7,7 @@
import java.nio.file.Paths; import java.nio.file.Paths;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;

import java.util.Objects;
import org.slf4j.Logger;


import com.google.common.base.Joiner; import com.google.common.base.Joiner;
import com.google.common.base.Optional; import com.google.common.base.Optional;
Expand All @@ -24,6 +23,8 @@
import com.hubspot.singularity.runner.base.shared.SimpleProcessManager; import com.hubspot.singularity.runner.base.shared.SimpleProcessManager;
import com.hubspot.singularity.runner.base.shared.TailMetadata; import com.hubspot.singularity.runner.base.shared.TailMetadata;


import org.slf4j.Logger;

public class SingularityExecutorTaskLogManager { public class SingularityExecutorTaskLogManager {


private final SingularityExecutorTaskDefinition taskDefinition; private final SingularityExecutorTaskDefinition taskDefinition;
Expand Down Expand Up @@ -162,7 +163,7 @@ private boolean writeTailMetadata(boolean finished) {
*/ */
private String getS3Glob() { private String getS3Glob() {
List<String> fileNames = new ArrayList<>(configuration.getS3UploaderAdditionalFiles()); List<String> fileNames = new ArrayList<>(configuration.getS3UploaderAdditionalFiles());
fileNames.add(taskDefinition.getServiceLogOutPath().getFileName().toString()); fileNames.add(Objects.toString(taskDefinition.getServiceLogOutPath().getFileName()));


return String.format("{%s}*.gz*", Joiner.on(",").join(fileNames)); return String.format("{%s}*.gz*", Joiner.on(",").join(fileNames));
} }
Expand All @@ -184,7 +185,12 @@ public Path getLogrotateConfPath() {
} }


private boolean writeS3MetadataFile(boolean finished) { private boolean writeS3MetadataFile(boolean finished) {
Path logrotateDirectory = taskDefinition.getServiceLogOutPath().getParent().resolve(configuration.getLogrotateToDirectory()); final Path serviceLogOutPath = taskDefinition.getServiceLogOutPath();
final Path parent = serviceLogOutPath.getParent();
if (parent == null) {
throw new IllegalStateException("S3 metadata file " + serviceLogOutPath + " has no parent");
}
final Path logrotateDirectory = parent.resolve(configuration.getLogrotateToDirectory());


S3UploadMetadata s3UploadMetadata = new S3UploadMetadata(logrotateDirectory.toString(), getS3Glob(), configuration.getS3UploaderBucket(), getS3KeyPattern(), finished, Optional.<String> absent(), Optional.<Integer> absent(), Optional.<String> absent(), S3UploadMetadata s3UploadMetadata = new S3UploadMetadata(logrotateDirectory.toString(), getS3Glob(), configuration.getS3UploaderBucket(), getS3KeyPattern(), finished, Optional.<String> absent(), Optional.<Integer> absent(), Optional.<String> absent(),
Optional.<String> absent(), Optional.<Long> absent()); Optional.<String> absent(), Optional.<Long> absent());
Expand Down
Expand Up @@ -12,6 +12,7 @@
import java.util.HashSet; import java.util.HashSet;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.Objects;
import java.util.Set; import java.util.Set;


import com.hubspot.singularity.executor.config.SingularityExecutorModule; import com.hubspot.singularity.executor.config.SingularityExecutorModule;
Expand Down Expand Up @@ -108,7 +109,7 @@ public SingularityExecutorCleanupStatistics clean() {
} }


for (Path file : JavaUtils.iterable(directory)) { for (Path file : JavaUtils.iterable(directory)) {
if (!file.getFileName().toString().endsWith(executorConfiguration.getGlobalTaskDefinitionSuffix())) { if (!Objects.toString(file.getFileName()).endsWith(executorConfiguration.getGlobalTaskDefinitionSuffix())) {
LOG.debug("Ignoring file {} that doesn't have suffix {}", file, executorConfiguration.getGlobalTaskDefinitionSuffix()); LOG.debug("Ignoring file {} that doesn't have suffix {}", file, executorConfiguration.getGlobalTaskDefinitionSuffix());
statisticsBldr.incrInvalidTasks(); statisticsBldr.incrInvalidTasks();
continue; continue;
Expand Down Expand Up @@ -215,7 +216,11 @@ private boolean cleanTask(SingularityExecutorTaskDefinition taskDefinition, Opti


private Iterator<Path> getUncompressedLogrotatedFileIterator(SingularityExecutorTaskDefinition taskDefinition) { private Iterator<Path> getUncompressedLogrotatedFileIterator(SingularityExecutorTaskDefinition taskDefinition) {
final Path serviceLogOutPath = taskDefinition.getServiceLogOutPath(); final Path serviceLogOutPath = taskDefinition.getServiceLogOutPath();
final Path logrotateToPath = taskDefinition.getServiceLogOutPath().getParent().resolve(executorConfiguration.getLogrotateToDirectory()); final Path parent = serviceLogOutPath.getParent();
if (parent == null) {
throw new IllegalStateException("Service log path " + serviceLogOutPath + " has no parent");
}
final Path logrotateToPath = parent.resolve(executorConfiguration.getLogrotateToDirectory());


if (!logrotateToPath.toFile().exists() || !logrotateToPath.toFile().isDirectory()) { if (!logrotateToPath.toFile().exists() || !logrotateToPath.toFile().isDirectory()) {
LOG.warn("Skipping uncompressed logrotated file cleanup for {} -- {} does not exist or is not a directory (task sandbox was probably garbage collected by Mesos)", taskDefinition.getTaskId(), logrotateToPath); LOG.warn("Skipping uncompressed logrotated file cleanup for {} -- {} does not exist or is not a directory (task sandbox was probably garbage collected by Mesos)", taskDefinition.getTaskId(), logrotateToPath);
Expand All @@ -240,14 +245,13 @@ private void checkForUncompressedLogrotatedFile(SingularityExecutorTaskDefinitio
while (iterator.hasNext()) { while (iterator.hasNext()) {
Path path = iterator.next(); Path path = iterator.next();


if (path.getFileName().toString().endsWith(".gz")) { final String fileName = Objects.toString(path.getFileName());
if (fileName.endsWith(".gz")) {
try { try {
if (Files.size(path) == 0) { if (Files.size(path) == 0) {
Files.deleteIfExists(path); Files.deleteIfExists(path);


String pathString = path.getFileName().toString(); emptyPaths.add(fileName.substring(0, fileName.length() - 3)); // removing .gz

emptyPaths.add(pathString.substring(0, pathString.length() - 3)); // removing .gz
} }
} catch (IOException ioe) { } catch (IOException ioe) {
LOG.error("Failed to handle empty gz file {}", path, ioe); LOG.error("Failed to handle empty gz file {}", path, ioe);
Expand All @@ -258,7 +262,7 @@ private void checkForUncompressedLogrotatedFile(SingularityExecutorTaskDefinitio
} }


for (Path path : ungzippedFiles) { for (Path path : ungzippedFiles) {
if (emptyPaths.contains(path.getFileName().toString())) { if (emptyPaths.contains(Objects.toString(path.getFileName()))) {
LOG.info("Gzipping abandoned file {}", path); LOG.info("Gzipping abandoned file {}", path);
try { try {
new SimpleProcessManager(LOG).runCommand(ImmutableList.<String> of("gzip", path.toString())); new SimpleProcessManager(LOG).runCommand(ImmutableList.<String> of("gzip", path.toString()));
Expand Down
Expand Up @@ -92,10 +92,13 @@ private void downloadAndCheck(RemoteArtifact artifact, Path downloadTo) {
public void extract(EmbeddedArtifact embeddedArtifact, Path directory) { public void extract(EmbeddedArtifact embeddedArtifact, Path directory) {
final Path extractTo = directory.resolve(embeddedArtifact.getFilename()); final Path extractTo = directory.resolve(embeddedArtifact.getFilename());


final Path parent = extractTo.getParent();
try { try {
Files.createDirectories(extractTo.getParent()); if (parent != null) {
Files.createDirectories(parent);
}
} catch (IOException e) { } catch (IOException e) {
throw new RuntimeException(String.format("Couldn't extract %s, unable to create directory %s", embeddedArtifact.getName(), extractTo.getParent()), e); throw new RuntimeException(String.format("Couldn't extract %s, unable to create directory %s", embeddedArtifact.getName(), parent), e);
} }


log.info("Extracting {} bytes of {} to {}", embeddedArtifact.getContent().length, embeddedArtifact.getName(), extractTo); log.info("Extracting {} bytes of {} to {}", embeddedArtifact.getContent().length, embeddedArtifact.getName(), extractTo);
Expand Down
Expand Up @@ -2,6 +2,7 @@


import java.nio.file.Path; import java.nio.file.Path;
import java.nio.file.Paths; import java.nio.file.Paths;
import java.util.Objects;


import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponse;


Expand Down Expand Up @@ -49,7 +50,7 @@ private void download() throws Exception {
return; return;
} }


if (fetched.getFileName().toString().endsWith(".tar.gz")) { if (Objects.toString(fetched.getFileName()).endsWith(".tar.gz")) {
artifactManager.untar(fetched, targetDirectory); artifactManager.untar(fetched, targetDirectory);
} else { } else {
artifactManager.copy(fetched, targetDirectory); artifactManager.copy(fetched, targetDirectory);
Expand Down
Expand Up @@ -9,6 +9,7 @@
import java.nio.file.Paths; import java.nio.file.Paths;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;
import java.util.Objects;
import java.util.Set; import java.util.Set;
import java.util.concurrent.Callable; import java.util.concurrent.Callable;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -198,8 +199,8 @@ private void uploadSingle(int sequence, Path file) throws Exception {
} }


class Uploader implements Callable<Boolean> { class Uploader implements Callable<Boolean> {
private int sequence; private final int sequence;
private Path file; private final Path file;


public Uploader(int sequence, Path file) { public Uploader(int sequence, Path file) {
this.file = file; this.file = file;
Expand All @@ -210,7 +211,7 @@ public Uploader(int sequence, Path file) {
public Boolean call() throws Exception { public Boolean call() throws Exception {
final long start = System.currentTimeMillis(); final long start = System.currentTimeMillis();


final String key = SingularityS3FormatHelper.getKey(uploadMetadata.getS3KeyFormat(), sequence, Files.getLastModifiedTime(file).toMillis(), file.getFileName().toString(), Optional.of(hostname)); final String key = SingularityS3FormatHelper.getKey(uploadMetadata.getS3KeyFormat(), sequence, Files.getLastModifiedTime(file).toMillis(), Objects.toString(file.getFileName()), Optional.of(hostname));


long fileSizeBytes = Files.size(file); long fileSizeBytes = Files.size(file);
LOG.info("{} Uploading {} to {}/{} (size {})", logIdentifier, file, s3Bucket.getName(), key, fileSizeBytes); LOG.info("{} Uploading {} to {}/{} (size {})", logIdentifier, file, s3Bucket.getName(), key, fileSizeBytes);
Expand Down

0 comments on commit cf057c0

Please sign in to comment.