Skip to content
This repository has been archived by the owner on Sep 12, 2024. It is now read-only.

Commit

Permalink
cli: job create: fix --env and -r/--register
Browse files Browse the repository at this point in the history
nargs causes argparse4j to consume flags and arguments following --env
and -r/--register. E.g. -r foo/tcp=http -p http=80 would cause -p and
http=80 to become appended as -r arguments into the registration list.

nargs is not useful here, so simply remove it.
  • Loading branch information
Daniel Norberg committed Jul 15, 2014
1 parent dd3ed07 commit 07c3cd1
Showing 1 changed file with 28 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ public JobCreateCommand(final Subparser parser) {
envArg = parser.addArgument("--env")
.action(append())
.setDefault(new ArrayList<String>())
.nargs("+")
.help("Environment variables");

portArg = parser.addArgument("-p", "--port")
Expand All @@ -125,7 +124,6 @@ public JobCreateCommand(final Subparser parser) {
registrationArg = parser.addArgument("-r", "--register")
.action(append())
.setDefault(new ArrayList<String>())
.nargs("+")
.help("Service discovery registration. Specify a service name, the port name and a " +
"protocol on the format service/protocol=port. E.g. -r website/tcp=http will " +
"register the port named http with the protocol tcp. Protocol is optional and " +
Expand Down Expand Up @@ -232,18 +230,16 @@ int run(Namespace options, HeliosClient client, PrintStream out, final boolean j
builder.setCommand(command);
}

final List<List<String>> envList = options.getList(envArg.getDest());
final List<String> envList = options.getList(envArg.getDest());
if (!envList.isEmpty()) {
final ImmutableMap.Builder<String, String> env = ImmutableMap.builder();
env.putAll(builder.getEnv());
for (final List<String> group : envList) {
for (final String s : group) {
final String[] parts = s.split("=", 2);
if (parts.length != 2) {
throw new IllegalArgumentException("Bad environment variable: " + s);
}
env.put(parts[0], parts[1]);
for (final String s : envList) {
final String[] parts = s.split("=", 2);
if (parts.length != 2) {
throw new IllegalArgumentException("Bad environment variable: " + s);
}
env.put(parts[0], parts[1]);
}
builder.setEnv(env.build());
}
Expand Down Expand Up @@ -280,35 +276,33 @@ int run(Namespace options, HeliosClient client, PrintStream out, final boolean j
final Map<ServiceEndpoint, ServicePorts> explicitRegistration = Maps.newHashMap();
final Pattern registrationPattern =
compile("(?<srv>[a-zA-Z][_\\-\\w]+)(?:/(?<prot>\\w+))?(?:=(?<port>[_\\-\\w]+))?");
final List<List<String>> registrationSpecLists = options.getList(registrationArg.getDest());
for (List<String> registrationSpecList : registrationSpecLists) {
for (final String spec : registrationSpecList) {
final Matcher matcher = registrationPattern.matcher(spec);
if (!matcher.matches()) {
throw new IllegalArgumentException("Bad registration: " + spec);
}
final List<String> registrationSpecs = options.getList(registrationArg.getDest());
for (final String spec : registrationSpecs) {
final Matcher matcher = registrationPattern.matcher(spec);
if (!matcher.matches()) {
throw new IllegalArgumentException("Bad registration: " + spec);
}

final String service = matcher.group("srv");
final String proto = fromNullable(matcher.group("prot")).or(HTTP);
final String optionalPort = matcher.group("port");
final String port;
final String service = matcher.group("srv");
final String proto = fromNullable(matcher.group("prot")).or(HTTP);
final String optionalPort = matcher.group("port");
final String port;

if (ports.size() == 0) {
throw new IllegalArgumentException("Need port mappings for service registration.");
}
if (ports.size() == 0) {
throw new IllegalArgumentException("Need port mappings for service registration.");
}

if (optionalPort == null) {
if (ports.size() != 1) {
throw new IllegalArgumentException(
"Need exactly one port mapping for implicit service registration");
}
port = Iterables.getLast(ports.keySet());
} else {
port = optionalPort;
if (optionalPort == null) {
if (ports.size() != 1) {
throw new IllegalArgumentException(
"Need exactly one port mapping for implicit service registration");
}

explicitRegistration.put(ServiceEndpoint.of(service, proto), ServicePorts.of(port));
port = Iterables.getLast(ports.keySet());
} else {
port = optionalPort;
}

explicitRegistration.put(ServiceEndpoint.of(service, proto), ServicePorts.of(port));
}

// Merge service registrations
Expand Down

0 comments on commit 07c3cd1

Please sign in to comment.