Repository: aurora Updated Branches: refs/heads/master 61e3a1aca -> bd90d768e
Remove support for reading command line argument values from files. Reviewed at https://reviews.apache.org/r/45936/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/bd90d768 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/bd90d768 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/bd90d768 Branch: refs/heads/master Commit: bd90d768e4fcddbbad36845f942346554e4d9012 Parents: 61e3a1a Author: Bill Farner <[email protected]> Authored: Fri Apr 8 12:20:54 2016 -0700 Committer: Bill Farner <[email protected]> Committed: Fri Apr 8 12:20:54 2016 -0700 ---------------------------------------------------------------------- RELEASE-NOTES.md | 6 +-- .../apache/aurora/common/args/ArgScanner.java | 11 ------ .../apache/aurora/common/args/ArgumentInfo.java | 11 ------ .../apache/aurora/common/args/OptionInfo.java | 37 +----------------- .../aurora/common/args/PositionalInfo.java | 2 +- .../aurora/common/args/OptionInfoTest.java | 40 -------------------- 6 files changed, 6 insertions(+), 101 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/bd90d768/RELEASE-NOTES.md ---------------------------------------------------------------------- diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 30e2275..a0536ec 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -1,4 +1,3 @@ - 0.13.0 (Not yet released) ------ @@ -56,8 +55,9 @@ - `addInstances` - `replaceCronTemplate` - Task ID strings are no longer prefixed by a timestamp. -- The scheduler previously supported specification of command line arguments by fully-qualified - class names. This support has been removed. +- Changes to the way the scheduler reads command line arguments + - Removed support for reading command line argument values from files. + - Removed support for specifying command line argument names with fully-qualified class names. 0.12.0 ------ http://git-wip-us.apache.org/repos/asf/aurora/blob/bd90d768/commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java ---------------------------------------------------------------------- diff --git a/commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java b/commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java index 46bd0c7..306ca4d 100644 --- a/commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java +++ b/commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java @@ -431,7 +431,6 @@ public final class ArgScanner { private void printHelp(Verifiers verifiers, ArgsInfo argsInfo) { ImmutableList.Builder<String> requiredHelps = ImmutableList.builder(); ImmutableList.Builder<String> optionalHelps = ImmutableList.builder(); - Optional<String> firstArgFileArgumentName = Optional.absent(); for (OptionInfo<?> optionInfo : ORDER_BY_NAME.immutableSortedCopy(argsInfo.getOptionInfos())) { Arg<?> arg = optionInfo.getArg(); @@ -443,9 +442,6 @@ public final class ArgScanner { } else { optionalHelps.add(help); } - if (optionInfo.argFile() && !firstArgFileArgumentName.isPresent()) { - firstArgFileArgumentName = Optional.of(optionInfo.getName()); - } } infoLog("-------------------------------------------------------------------------"); @@ -478,13 +474,6 @@ public final class ArgScanner { infoLog("\nOptional flags:"); infoLog(Joiner.on('\n').join(optional)); } - if (firstArgFileArgumentName.isPresent()) { - infoLog(String.format("\n" - + "For arguments that support @argfile format: @argfile is a text file that contains " - + "cmdline argument values. For example: -%s=@/tmp/%s_value.txt. The format " - + "of the argfile content should be exactly the same as it would be specified on the " - + "cmdline.", firstArgFileArgumentName.get(), firstArgFileArgumentName.get())); - } infoLog("-------------------------------------------------------------------------"); } http://git-wip-us.apache.org/repos/asf/aurora/blob/bd90d768/commons/src/main/java/org/apache/aurora/common/args/ArgumentInfo.java ---------------------------------------------------------------------- diff --git a/commons/src/main/java/org/apache/aurora/common/args/ArgumentInfo.java b/commons/src/main/java/org/apache/aurora/common/args/ArgumentInfo.java index e28bae2..a59d109 100644 --- a/commons/src/main/java/org/apache/aurora/common/args/ArgumentInfo.java +++ b/commons/src/main/java/org/apache/aurora/common/args/ArgumentInfo.java @@ -62,7 +62,6 @@ public abstract class ArgumentInfo<T> { private final String name; private final String help; - private final boolean argFile; private final Arg<T> arg; private final TypeToken<T> type; private final List<Annotation> verifierAnnotations; @@ -73,7 +72,6 @@ public abstract class ArgumentInfo<T> { * * @param name The simple name for the argument. * @param help Help string. - * @param argFile If argument file is allowed. * @param arg Argument object. * @param type Concrete argument type. * @param verifierAnnotations {@link Verifier} annotations for this @@ -83,7 +81,6 @@ public abstract class ArgumentInfo<T> { protected ArgumentInfo( String name, String help, - boolean argFile, Arg<T> arg, TypeToken<T> type, List<Annotation> verifierAnnotations, @@ -91,7 +88,6 @@ public abstract class ArgumentInfo<T> { this.name = MorePreconditions.checkNotBlank(name); this.help = MorePreconditions.checkNotBlank(help); - this.argFile = argFile; this.arg = Preconditions.checkNotNull(arg); this.type = Preconditions.checkNotNull(type); this.verifierAnnotations = ImmutableList.copyOf(verifierAnnotations); @@ -116,13 +112,6 @@ public abstract class ArgumentInfo<T> { } /** - * Returns whether an argument file is allowed for this argument. - */ - public boolean argFile() { - return argFile; - } - - /** * Returns the Arg associated with this command-line argument. The Arg<?> is a mutable container * cell that holds the value passed-in on the command line, after parsing and validation. */ http://git-wip-us.apache.org/repos/asf/aurora/blob/bd90d768/commons/src/main/java/org/apache/aurora/common/args/OptionInfo.java ---------------------------------------------------------------------- diff --git a/commons/src/main/java/org/apache/aurora/common/args/OptionInfo.java b/commons/src/main/java/org/apache/aurora/common/args/OptionInfo.java index 36a472d..2fcd3e8 100644 --- a/commons/src/main/java/org/apache/aurora/common/args/OptionInfo.java +++ b/commons/src/main/java/org/apache/aurora/common/args/OptionInfo.java @@ -13,8 +13,6 @@ */ package org.apache.aurora.common.args; -import java.io.File; -import java.io.IOException; import java.lang.annotation.Annotation; import java.lang.reflect.Field; import java.util.Arrays; @@ -23,11 +21,8 @@ import java.util.regex.Pattern; import javax.annotation.Nullable; -import com.google.common.base.Charsets; import com.google.common.base.Optional; import com.google.common.base.Preconditions; -import com.google.common.base.Strings; -import com.google.common.io.Files; import com.google.common.reflect.TypeToken; import org.apache.aurora.common.args.apt.Configuration; @@ -44,7 +39,6 @@ public final class OptionInfo<T> extends ArgumentInfo<T> { private static final Pattern ARG_NAME_PATTERN = Pattern.compile(ARG_NAME_RE); private static final String NEGATE_BOOLEAN = "no_"; - private static final String ARG_FILE_INDICATOR = "@"; /** * Factory method to create a OptionInfo from a field. @@ -111,7 +105,7 @@ public final class OptionInfo<T> extends ArgumentInfo<T> { List<Annotation> verifierAnnotations, @Nullable Class<? extends Parser<T>> parser) { - super(name, help, argFile, arg, type, verifierAnnotations, parser); + super(name, help, arg, type, verifierAnnotations, parser); } /** @@ -120,17 +114,7 @@ public final class OptionInfo<T> extends ArgumentInfo<T> { void load(ParserOracle parserOracle, String optionName, String value) { Parser<? extends T> parser = getParser(parserOracle); - String finalValue = value; - - // If "-arg=@file" is allowed and specified, then we read the value from the file - // and use it as the raw value to be parsed for the argument. - if (argFile() - && !Strings.isNullOrEmpty(value) - && value.startsWith(ARG_FILE_INDICATOR)) { - finalValue = getArgFileContent(optionName, value.substring(ARG_FILE_INDICATOR.length())); - } - - Object result = parser.parse(parserOracle, getType().getType(), finalValue); // [A] + Object result = parser.parse(parserOracle, getType().getType(), value); // [A] // If the arg type is boolean, check if the command line uses the negated boolean form. if (isBoolean()) { @@ -158,21 +142,4 @@ public final class OptionInfo<T> extends ArgumentInfo<T> { String getNegatedName() { return NEGATE_BOOLEAN + getName(); } - - private String getArgFileContent(String optionName, String argFilePath) - throws IllegalArgumentException { - if (argFilePath.isEmpty()) { - throw new IllegalArgumentException( - String.format("Invalid null/empty value for argument '%s'.", optionName)); - } - - try { - return Files.toString(new File(argFilePath), Charsets.UTF_8); - } catch (IOException e) { - throw new IllegalArgumentException( - String.format("Unable to read argument '%s' value from file '%s'.", - optionName, argFilePath), - e); - } - } } http://git-wip-us.apache.org/repos/asf/aurora/blob/bd90d768/commons/src/main/java/org/apache/aurora/common/args/PositionalInfo.java ---------------------------------------------------------------------- diff --git a/commons/src/main/java/org/apache/aurora/common/args/PositionalInfo.java b/commons/src/main/java/org/apache/aurora/common/args/PositionalInfo.java index 466b8e2..83d9cd5 100644 --- a/commons/src/main/java/org/apache/aurora/common/args/PositionalInfo.java +++ b/commons/src/main/java/org/apache/aurora/common/args/PositionalInfo.java @@ -92,7 +92,7 @@ public final class PositionalInfo<T> extends ArgumentInfo<List<T>> { // TODO: https://github.com/twitter/commons/issues/353, consider future support of // argFile for Positional arguments. - super(name, help, false, arg, type, verifierAnnotations, parser); + super(name, help, arg, type, verifierAnnotations, parser); this.elementType = elementType; } http://git-wip-us.apache.org/repos/asf/aurora/blob/bd90d768/commons/src/test/java/org/apache/aurora/common/args/OptionInfoTest.java ---------------------------------------------------------------------- diff --git a/commons/src/test/java/org/apache/aurora/common/args/OptionInfoTest.java b/commons/src/test/java/org/apache/aurora/common/args/OptionInfoTest.java index 58890ab..7573430 100644 --- a/commons/src/test/java/org/apache/aurora/common/args/OptionInfoTest.java +++ b/commons/src/test/java/org/apache/aurora/common/args/OptionInfoTest.java @@ -16,9 +16,7 @@ package org.apache.aurora.common.args; import java.io.File; import java.util.List; -import com.google.common.base.Charsets; import com.google.common.collect.ImmutableList; -import com.google.common.io.Files; import org.junit.Before; import org.junit.Rule; @@ -26,8 +24,6 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; import static junit.framework.Assert.assertEquals; -import static junit.framework.Assert.assertFalse; -import static junit.framework.Assert.assertTrue; public class OptionInfoTest { private static class App { @@ -48,16 +44,6 @@ public class OptionInfoTest { } @Test - public void testArgumentFilesCreateFromField() throws Exception { - OptionInfo optionInfo = OptionInfo.createFromField(App.class.getDeclaredField("files"), app); - assertEquals("files", optionInfo.getName()); - assertEquals( - String.format(OptionInfo.ARG_FILE_HELP_TEMPLATE, "help.", "files", "files"), - optionInfo.getHelp()); - assertTrue(optionInfo.argFile()); - } - - @Test public void testArgumentFilesRegularFormat() throws Exception { new ArgScanner().parse(Args.from(ArgFilters.selectClass(App.class), app), ImmutableList.of("-files=1.txt,2.txt")); @@ -66,37 +52,11 @@ public class OptionInfoTest { app.files.get()); } - @Test(expected = IllegalArgumentException.class) - public void testArgumentFilesArgFileFormatEmptyFileName() throws Exception { - new ArgScanner().parse(Args.from(ArgFilters.selectClass(App.class), app), - ImmutableList.of("-files=@")); - } - - @Test(expected = IllegalArgumentException.class) - public void testArgumentFilesArgFileFormatFileNotExist() throws Exception { - new ArgScanner().parse(Args.from(ArgFilters.selectClass(App.class), app), - ImmutableList.of("-files=@file_does_not_exist.txt")); - } - - @Test - public void testArgumentFilesArgFileFormat() throws Exception { - File argfile = tmpDir.newFile(); - // Note the '\n' at the end. Some editors auto add a newline at the end so - // make sure our arg scanner and parser can deal with this. - Files.write("1.txt,2.txt\n", argfile, Charsets.UTF_8); - new ArgScanner().parse(Args.from(ArgFilters.selectClass(App.class), app), - ImmutableList.of("-files=@" + argfile.getCanonicalPath())); - assertEquals( - ImmutableList.of(new File("1.txt"), new File("2.txt")), - app.files.get()); - } - @Test public void testArgumentFlagCreateFromField() throws Exception { OptionInfo optionInfo = OptionInfo.createFromField(App.class.getDeclaredField("flag"), app); assertEquals("flag", optionInfo.getName()); assertEquals("help.", optionInfo.getHelp()); - assertFalse(optionInfo.argFile()); assertEquals("no_flag", optionInfo.getNegatedName()); } }
