Skip to content

Commit

Permalink
Provide a better error message for non-existing gcpTempLocation
Browse files Browse the repository at this point in the history
gcpTempLocation will default to using the value for tmpLocation, as long
as the value is a valid GCP path. Non-valid GCP paths are silently
discarded.

This change removes existence validation from the default value logic
such that downstream validation can provide a better error message.
  • Loading branch information
swegner committed Dec 6, 2016
1 parent c72708c commit 9d768df
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.beam.runners.dataflow;

import static org.apache.beam.sdk.util.WindowedValue.valueInGlobalWindow;
import static org.hamcrest.Matchers.both;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -255,6 +256,26 @@ public void testPathValidation() {
}
}

@Test
public void testPathExistsValidation() {
String[] args = new String[] {
"--runner=DataflowRunner",
"--tempLocation=gs://does/not/exist",
"--project=test-project",
"--credentialFactoryClass=" + NoopCredentialFactory.class.getCanonicalName(),
};

try {
TestPipeline.fromOptions(PipelineOptionsFactory.fromArgs(args).create());
fail();
} catch (RuntimeException e) {
assertThat(
Throwables.getStackTraceAsString(e),
both(containsString("gs://does/not/exist"))
.and(containsString("does not exist or is not writeable")));
}
}

@Test
public void testPathValidatorOverride() {
String[] args = new String[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ public String create(PipelineOptions options) {
if (!Strings.isNullOrEmpty(tempLocation)) {
try {
PathValidator validator = options.as(GcsOptions.class).getPathValidator();
validator.validateOutputFilePrefixSupported(tempLocation);
validator.verifyPath(tempLocation);
} catch (Exception e) {
// Ignore the temp location because it is not a valid 'gs://' path.
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,16 @@ public void testDefaultGcpTempLocationInvalid() throws Exception {
assertTrue(isNullOrEmpty(options.getGcpTempLocation()));
}

@Test
public void testDefaultGcpTempLocationDoesNotExist() throws Exception {
GcpOptions options = PipelineOptionsFactory.as(GcpOptions.class);
String tempLocation = "gs://does/not/exist";
options.setTempLocation(tempLocation);
String msg = "gcpTempLocation should accept non-existing default tempLocation so downstream "
+ "validation can provide a better error message";
assertEquals(msg, tempLocation, options.getGcpTempLocation());
}

private static void makePropertiesFileWithProject(File path, String projectId)
throws IOException {
String properties = String.format("[core]%n"
Expand Down

0 comments on commit 9d768df

Please sign in to comment.