Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add shell support for task params #525

Closed
wants to merge 1 commit into from

Conversation

jvalkeal
Copy link
Collaborator

  • Modify shell 'task launch' command to accept raw
    'params' option for java fatjar arguments.
  • Modify TaskDeploymentController to accept a list of arguments
    to be passed into SPI's AppDeploymentRequest.
  • Added one test and had to modify mockito ArgumentCaptor to accept
    'atLeast' as otherwise other tests will capture same calls to deployer.
  • Fixes Add command line args support for tasks #503

Testing manually:

Having definition

dataflow:>task create --name footask --definition "timestamp"

normal launch will output

dataflow:>task launch --name footask
2016-04-15 18:32:49.933  INFO 20878 --- [           main] o.s.c.t.t.TaskApplication$TimestampTask  : 2016-04-15 18:32:49.933

defining format via params

dataflow:>task launch --name footask --params "--format=yyyy-MM-dd"
2016-04-15 18:32:12.948  INFO 20815 --- [           main] o.s.c.t.t.TaskApplication$TimestampTask  : 2016-04-15

- Modify shell 'task launch' command to accept raw
  'params' option for java fatjar arguments.
- Modify TaskDeploymentController to accept a list of arguments
  to be passed into SPI's `AppDeploymentRequest`.
- Added one test and had to modify mockito ArgumentCaptor to accept
  'atLeast' as otherwise other tests will capture same calls to deployer.
- Fixes spring-cloud#503
@@ -108,9 +110,10 @@ public TaskDefinitionResource create(String name, String definition) {
}

@Override
public void launch(String name, Map<String, String> properties) {
public void launch(String name, Map<String, String> properties, List<String> arguments) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpicky but we can call these parameters.

Copy link
Collaborator Author

@jvalkeal jvalkeal Apr 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, removed wrong comments, called arguments as those go to 'java' arguments which are different than options. also in SPI those are called arguments to align same naming, but in shell params as requested.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change needs to be carried through out the PR.

@cppwfs
Copy link
Contributor

cppwfs commented Apr 15, 2016

may want to update the docs to state we support --params

@jvalkeal
Copy link
Collaborator Author

I'll check what's going on with this PR and update it to merge cleanly with current master.

@jvalkeal
Copy link
Collaborator Author

Closing this PR to make a new one post release.

@jvalkeal jvalkeal removed the in pr label Apr 26, 2016
@jvalkeal jvalkeal closed this Apr 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants