Github user chtyim commented on a diff in the pull request:
https://github.com/apache/incubator-twill/pull/24#discussion_r24641000
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/yarn/AbstractYarnProcessLauncher.java
---
@@ -113,108 +114,57 @@ private void addLocalFile(LocalFile localFile) {
}
@Override
- public ResourcesAdder withResources() {
- return new MoreResourcesImpl();
+ public PrepareLaunchContext addResources(LocalFile... localFiles) {
+ return addResources(Arrays.asList(localFiles));
}
@Override
- public AfterResources noResources() {
- return new MoreResourcesImpl();
- }
-
- private final class MoreResourcesImpl implements MoreResources {
-
- @Override
- public MoreResources add(LocalFile localFile) {
+ public PrepareLaunchContext addResources(Iterable<LocalFile>
localFiles) {
+ for (LocalFile localFile : localFiles) {
addLocalFile(localFile);
- return this;
- }
-
- @Override
- public EnvironmentAdder withEnvironment() {
- return finish();
- }
-
- @Override
- public AfterEnvironment noEnvironment() {
- return finish();
- }
-
- private MoreEnvironmentImpl finish() {
- launchContext.setLocalResources(localResources);
- return new MoreEnvironmentImpl();
}
+ return this;
}
- private final class MoreEnvironmentImpl implements MoreEnvironment {
-
- @Override
- public CommandAdder withCommands() {
- launchContext.setEnvironment(environment);
- return new MoreCommandImpl();
- }
-
- @Override
- public <V> MoreEnvironment add(String key, V value) {
- environment.put(key, value.toString());
- return this;
- }
+ @Override
+ public <V> PrepareLaunchContext addEnvironment(String key, V value) {
+ environment.put(key, value.toString());
+ return this;
}
- private final class MoreCommandImpl implements MoreCommand,
StdOutSetter, StdErrSetter {
-
- private final StringBuilder commandBuilder = new StringBuilder();
-
- @Override
- public StdOutSetter add(String cmd, String... args) {
- commandBuilder.append(cmd);
- for (String arg : args) {
- commandBuilder.append(' ').append(arg);
- }
- return this;
- }
-
- @Override
- public <R> ProcessController<R> launch() {
- if (credentials != null && !credentials.getAllTokens().isEmpty()) {
- for (Token<?> token : credentials.getAllTokens()) {
- LOG.info("Launch with delegation token {}", token);
- }
- launchContext.setCredentials(credentials);
- }
- launchContext.setCommands(commands);
- return doLaunch(launchContext);
+ @Override
+ public PrepareLaunchContext addCommand(String cmd, String... args) {
+ StringBuilder builder = new StringBuilder(cmd);
+ for (String arg : args) {
+ builder.append(' ').append(arg);
}
- @Override
- public MoreCommand redirectError(String stderr) {
- redirect(2, stderr);
- return noError();
- }
+ // Redirect stdout and stderr
+ redirect(1, ApplicationConstants.STDOUT, builder);
+ redirect(2, ApplicationConstants.STDERR, builder);
- @Override
- public MoreCommand noError() {
- commands.add(commandBuilder.toString());
- commandBuilder.setLength(0);
- return this;
- }
-
- @Override
- public StdErrSetter redirectOutput(String stdout) {
- redirect(1, stdout);
- return this;
- }
+ commands.add(builder.toString());
+ return this;
+ }
- @Override
- public StdErrSetter noOutput() {
- return this;
+ @Override
+ public <R> ProcessController<R> launch() {
+ launchContext.setLocalResources(localResources);
+ launchContext.setEnvironment(environment);
+ if (credentials != null && !credentials.getAllTokens().isEmpty()) {
+ for (Token<?> token : credentials.getAllTokens()) {
+ LOG.info("Launch with delegation token {}", token);
--- End diff --
Maybe it's better to just log the type and id explicitly, without relying
on `Token.toString()` to do it right.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
