aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wfar...@apache.org
Subject aurora git commit: Remove support for reading command line argument values from files.
Date Fri, 08 Apr 2016 19:20:37 GMT
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 <wfarner@apache.org>
Authored: Fri Apr 8 12:20:54 2016 -0700
Committer: Bill Farner <wfarner@apache.org>
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());
   }
 }


Mime
View raw message