Skip to content

Commit

Permalink
Fixing a bug where the publish method could not be given on the command
Browse files Browse the repository at this point in the history 
line if a control file was used; also tidying up the error messaging.
  • Loading branch information
ayn leslie committed Dec 31, 2014
1 parent f48d06a commit 24b20f0
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 31 deletions.
8 changes: 4 additions & 4 deletions src/main/java/com/socrata/datasync/job/IntegrationJob.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,11 @@ public JobStatus run() {
File fileToPublishFile = new File(fileToPublish);
if (publishViaDi2Http) {
try (DeltaImporter2Publisher publisher = new DeltaImporter2Publisher(userPrefs)) {
String action = controlFile.action == null ? publishMethod.name() : controlFile.action;
// "upsert" == "append" in di2
if ("upsert".equalsIgnoreCase(controlFile.action))
controlFile.action = "Append";
// TODO: remove the next line when di2 is updated to accept lowercase variants
controlFile.action = Utils.capitalizeFirstLetter(controlFile.action);
if ("upsert".equalsIgnoreCase(action))
action = "Append";
controlFile.action = Utils.capitalizeFirstLetter(action);
runStatus = publisher.publishWithDi2OverHttp(datasetID, fileToPublishFile, controlFile);
}
} else if (publishViaFTP) {
Expand Down
63 changes: 36 additions & 27 deletions src/main/java/com/socrata/datasync/validation/IntegrationJobValidity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public static JobStatus validateJobParams(UserPreferences userPrefs, Integration

if (rawHeaders == null) {
JobStatus noHeaders = JobStatus.PUBLISH_ERROR;
noHeaders.setMessage("Headers must be specified in one of " + publishFile.getName() + " or the control file using 'columns'");
noHeaders.setMessage("Headers must be specified in one of " + publishFile.getName() + " or the control file using 'columns'.");
return noHeaders;
}

Expand Down Expand Up @@ -148,7 +148,7 @@ private static boolean validatePathToControlFileArg(CommandLine cmd, CommandLine
System.err.println("Invalid argument: Neither -sc,--" + options.PATH_TO_FTP_CONTROL_FILE_FLAG +
" -cf,--" + options.PATH_TO_CONTROL_FILE_FLAG + " can be supplied " +
"unless -pf,--" + options.PUBLISH_VIA_FTP_FLAG + " is 'true' or " +
"unless -ph,--" + options.PUBLISH_VIA_DI2_FLAG + " is 'true'");
"unless -ph,--" + options.PUBLISH_VIA_DI2_FLAG + " is 'true'.");
return false;
}
}
Expand All @@ -157,7 +157,7 @@ private static boolean validatePathToControlFileArg(CommandLine cmd, CommandLine
if(cmd.getOptionValue(options.HAS_HEADER_ROW_FLAG) != null) {
System.out.println("WARNING: -h,--" + options.HAS_HEADER_ROW_FLAG + " is being ignored because " +
"-sc,--" + options.PATH_TO_FTP_CONTROL_FILE_FLAG + " or " +
"-cf,--" + options.PATH_TO_CONTROL_FILE_FLAG + " was supplied");
"-cf,--" + options.PATH_TO_CONTROL_FILE_FLAG + " was supplied.");
}
}
return true;
Expand All @@ -169,21 +169,21 @@ private static boolean validatePublishViaFtpArg(CommandLine cmd, CommandLineOpti
return true;

if (!publishingWithFtp.equalsIgnoreCase("true") && !publishingWithFtp.equalsIgnoreCase("false")) {
System.err.println("Invalid argument: -pf,--" + options.PUBLISH_VIA_FTP_FLAG + " must be 'true' or 'false'");
System.err.println("Invalid argument: -pf,--" + options.PUBLISH_VIA_FTP_FLAG + " must be 'true' or 'false'.");
return false;
}
if (publishingWithFtp.equalsIgnoreCase("true")) {
String publishingWithDi2 = cmd.getOptionValue(options.PUBLISH_VIA_DI2_FLAG);
if (publishingWithDi2 != null && publishingWithDi2.equalsIgnoreCase("true")) {
System.err.println("Only one of -pf,--" + options.PUBLISH_VIA_DI2_FLAG + " and " +
"-ph,--" + options.PUBLISH_VIA_DI2_FLAG + " may be set to 'true'");
"-ph,--" + options.PUBLISH_VIA_DI2_FLAG + " may be set to 'true'.");
return false;
}
String controlFilePath = cmd.getOptionValue(options.PATH_TO_CONTROL_FILE_FLAG);
if (controlFilePath == null) controlFilePath = cmd.getOptionValue(options.PATH_TO_FTP_CONTROL_FILE_FLAG);
if (controlFilePath == null) {
System.err.println("A control file must be specified when " +
"-pf,--" + options.PUBLISH_VIA_FTP_FLAG + " is set to 'true'");
"-pf,--" + options.PUBLISH_VIA_FTP_FLAG + " is set to 'true'.");
return false;
}
}
Expand All @@ -196,21 +196,21 @@ private static boolean validatePublishViaDi2HttpArg(CommandLine cmd, CommandLine
return true;

if(!publishingWithDi2.equalsIgnoreCase("true") && !publishingWithDi2.equalsIgnoreCase("false")) {
System.err.println("Invalid argument: -pf,--" + options.PUBLISH_VIA_DI2_FLAG + " must be 'true' or 'false'");
System.err.println("Invalid argument: -pf,--" + options.PUBLISH_VIA_DI2_FLAG + " must be 'true' or 'false'.");
return false;
}
if (publishingWithDi2.equalsIgnoreCase("true")) {
String publishingWithFtp = cmd.getOptionValue(options.PUBLISH_VIA_FTP_FLAG);
if (publishingWithFtp != null && publishingWithFtp.equalsIgnoreCase("true")) {
System.err.println("Only one of -pf,--" + options.PUBLISH_VIA_DI2_FLAG + " and " +
"-ph,--" + options.PUBLISH_VIA_DI2_FLAG + " may be set to 'true'");
"-ph,--" + options.PUBLISH_VIA_DI2_FLAG + " may be set to 'true'.");
return false;
}
String controlFilePath = cmd.getOptionValue(options.PATH_TO_CONTROL_FILE_FLAG);
if (controlFilePath == null) controlFilePath = cmd.getOptionValue(options.PATH_TO_FTP_CONTROL_FILE_FLAG);
if (controlFilePath == null) {
System.err.println("A control file must be specified when " +
"-ph,--" + options.PUBLISH_VIA_DI2_FLAG + " is set to 'true'");
"-ph,--" + options.PUBLISH_VIA_DI2_FLAG + " is set to 'true'.");
return false;
}
}
Expand All @@ -225,7 +225,7 @@ private static boolean validateHeaderRowArg(CommandLine cmd, CommandLineOptions
if(haveHeader == null) {
if (controlFilePath == null) {
if (isNullOrFalse(publishingWithFtp) && isNullOrFalse(publishingWithDi2)) {
System.err.println("Missing required argument: -h,--" + options.HAS_HEADER_ROW_FLAG + " is required");
System.err.println("Missing required argument: -h,--" + options.HAS_HEADER_ROW_FLAG + " is required.");
return false;
} else {
// if publishing via ftp or di2/http, we want to err about the control file, not the header arg
Expand All @@ -237,7 +237,7 @@ private static boolean validateHeaderRowArg(CommandLine cmd, CommandLineOptions
} else { // have non-null header arg
if (!cmd.getOptionValue(options.HAS_HEADER_ROW_FLAG).equalsIgnoreCase("true")
&& !cmd.getOptionValue(options.HAS_HEADER_ROW_FLAG).equalsIgnoreCase("false")) {
System.err.println("Invalid argument: -h,--" + options.HAS_HEADER_ROW_FLAG + " must be 'true' or 'false'");
System.err.println("Invalid argument: -h,--" + options.HAS_HEADER_ROW_FLAG + " must be 'true' or 'false'.");
return false;
}
return true;
Expand All @@ -246,15 +246,15 @@ private static boolean validateHeaderRowArg(CommandLine cmd, CommandLineOptions

private static boolean validateFileToPublishArg(CommandLine cmd, CommandLineOptions options) {
if(cmd.getOptionValue(options.FILE_TO_PUBLISH_FLAG) == null) {
System.err.println("Missing required argument: -f,--" + options.FILE_TO_PUBLISH_FLAG + " is required");
System.err.println("Missing required argument: -f,--" + options.FILE_TO_PUBLISH_FLAG + " is required.");
return false;
}
return true;
}

private static boolean validateDatasetIdArg(CommandLine cmd, CommandLineOptions options) {
if(cmd.getOptionValue(options.DATASET_ID_FLAG) == null) {
System.err.println("Missing required argument: -i,--" + options.DATASET_ID_FLAG + " is required");
System.err.println("Missing required argument: -i,--" + options.DATASET_ID_FLAG + " is required.");
return false;
}
return true;
Expand All @@ -268,7 +268,7 @@ private static boolean validatePublishMethodArg(CommandLine cmd, CommandLineOpti
if(method == null) {
if (controlFilePath == null) {
if (isNullOrFalse(publishingWithFtp) && isNullOrFalse(publishingWithDi2)) {
System.err.println("Missing required argument: -m,--" + options.PUBLISH_METHOD_FLAG + " is required");
System.err.println("Missing required argument: -m,--" + options.PUBLISH_METHOD_FLAG + " is required.");
return false;
} else {
// if publishing via ftp or di2/http, we want to err about the control file, not the method arg
Expand All @@ -284,8 +284,8 @@ private static boolean validatePublishMethodArg(CommandLine cmd, CommandLineOpti
publishMethodValid = true;
}
if (!publishMethodValid) {
System.err.println("Invalid argument: -m,--" + options.PUBLISH_METHOD_FLAG + " must be " +
Arrays.toString(PublishMethod.values()));
System.err.println("Invalid argument: -m,--" + options.PUBLISH_METHOD_FLAG + " must be one of " +
Arrays.toString(PublishMethod.values()) + ".");
return false;
}
return true;
Expand All @@ -297,11 +297,11 @@ private static boolean validateProxyArgs(CommandLine cmd, CommandLineOptions opt
String password = cmd.getOptionValue(options.PROXY_PASSWORD_FLAG);
if(username == null && password != null) {
System.err.println("Missing required argument: -pun,--" + options.PROXY_USERNAME_FLAG + " is required if" +
" supplying proxy credentials with -ppw, --" + options.PROXY_PASSWORD_FLAG);
" supplying proxy credentials with -ppw, --" + options.PROXY_PASSWORD_FLAG + ".");
return false;
} else if(username != null && password == null) {
System.err.println("Missing required argument: -ppw,--" + options.PROXY_PASSWORD_FLAG + " is required if" +
" supplying proxy credentials with -pun, --" + options.PROXY_USERNAME_FLAG);
" supplying proxy credentials with -pun, --" + options.PROXY_USERNAME_FLAG + ".");
return false;
}
return true;
Expand Down Expand Up @@ -372,8 +372,16 @@ private static JobStatus validateControlFile(FileTypeControl fileControl, String
}

private static JobStatus checkAction(String action, IntegrationJob job, Dataset schema) {
if (action == null && job.getPublishMethod() == null) {
JobStatus status = JobStatus.PUBLISH_ERROR;
status.setMessage("Unknown Publish Method: " +
"A publish method must either be specified in the control file via the 'action' field or " +
"on the command line using the -m,--publishMethod option.");
return status;
}
StringBuilder methods = new StringBuilder();
boolean okAction = false;
if (action == null) action = job.getPublishMethod().name();
for (PublishMethod m : PublishMethod.values()) {
methods.append("\t" + m.name() + "\n");
if (m.name().equalsIgnoreCase(action))
Expand All @@ -383,28 +391,28 @@ private static JobStatus checkAction(String action, IntegrationJob job, Dataset
JobStatus status = JobStatus.PUBLISH_ERROR;
status.setMessage("Unknown Publish Method: " +
"The control file must specify the publishing method via the 'action' option as one of: \n" +
methods.toString() + "\n The action provided was '" + action + "'");
methods.toString() + "\n The action provided was '" + action + "'.");
return status;
}
if (!PublishMethod.replace.name().equalsIgnoreCase(action) && job.getPublishViaFTP()) {
JobStatus status = JobStatus.PUBLISH_ERROR;
status.setMessage("FTP does not currently support upsert, append or delete");
status.setMessage("FTP does not currently support upsert, append or delete.");
return status;
}
PublishMethod publishMethod = job.getPublishMethod();
if (publishMethod != null && !action.equalsIgnoreCase(publishMethod.name())) {
JobStatus status = JobStatus.PUBLISH_ERROR;
status.setMessage("Conflicting Publish Methods: " +
"The publish method selected was '" + publishMethod.name() +
"', but the 'action' option in the control file specifies the publish method as '" + action + ".");
"', but the 'action' option in the control file specifies the publish method as '" + action + "'.");
return status;
}
String rowIdentifier = DatasetUtils.getRowIdentifierName(schema);
if (rowIdentifier == null && PublishMethod.delete.name().equalsIgnoreCase(action)) {
JobStatus status = JobStatus.PUBLISH_ERROR;
status.setMessage("Dataset Requirement Unfulfilled: " +
"To delete from a dataset, a row identifier must be set. Dataset '" + schema.getId() +
"' does not have a row identifier set");
"' does not have a row identifier set.");
return status;
}
return JobStatus.VALID;
Expand All @@ -424,7 +432,7 @@ private static JobStatus checkTimeFormattingValidity(FileTypeControl fileControl
JobStatus status = JobStatus.PUBLISH_ERROR;
status.setMessage("Unsupported Date Time Format: The time format '" + format +
"' specified in the control file is not a valid pattern." +
"\nPlease consult " + jodaLink + " for more information");
"\nPlease consult " + jodaLink + " for more information.");
return status;
}
}
Expand Down Expand Up @@ -456,7 +464,7 @@ private static JobStatus checkEncodingValidity(FileTypeControl fileControl, Stri
if (!encodingFound) {
JobStatus status = JobStatus.PUBLISH_ERROR;
status.setMessage("Unsupported Encoding: The encoding '" + encoding + "' in the control file is not supported." +
"\nPlease consult " + charsetUri + " for a listing of supported encodings");
"\nPlease consult " + charsetUri + " for a listing of supported encodings.");
return status;
}
} catch (Exception e) {
Expand Down Expand Up @@ -510,8 +518,9 @@ private static JobStatus checkControlAgreement(FileTypeControl fileControl, Data
status.setMessage("Synthetic Location Not Found: The synthetic location column '" + field +
"' references a component '" + locationComponents[i] + "' which is not present in '" +
csvFilename + "'." +
"\nPlease check your control file to ensure that the column name is spelled correctly, " +
"and that '" + locationComponents[i] + "' is not included in the 'ignoreColumns' array.");
"\nPlease check your control file to ensure that the column name is spelled correctly " +
"and that headers are provided in either the file to publish or " +
"in the 'columns' field of the control file.");
return status;
}
}
Expand Down Expand Up @@ -610,7 +619,7 @@ private static JobStatus checkColumnAgreement(Dataset schema, String[] finalHead
"', but dataset '" + schema.getId() + "' does not." +
"\nPlease check that your headers are using the field name rather than the human readable name." +
"\nConsider using 'ignoreColumns' in the control file if '" + field +
"' should not be included in the dataset");
"' should not be included in the dataset.");
return status;
}
} else {
Expand Down

0 comments on commit 24b20f0

Please sign in to comment.