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.

Sign up for GitHub

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

Jump to bottom

Add command line args support for tasks #503

Closed
jvalkeal opened this issue Apr 8, 2016 · 7 comments
Closed

Add command line args support for tasks #503

jvalkeal opened this issue Apr 8, 2016 · 7 comments
Assignees
@jvalkeal
Milestone
1.0.0.RC1

Comments

@jvalkeal
Copy link
Collaborator

jvalkeal commented Apr 8, 2016 •
edited by sabbyanandan

When work for spring-cloud/spring-cloud-deployer#20 is done, its wingman here needs to similar things in a shell and controllers.
@jvalkeal jvalkeal added the ready label Apr 8, 2016
@jvalkeal jvalkeal self-assigned this Apr 8, 2016
@sabbyanandan sabbyanandan added this to the 1.0.0.M3 milestone Apr 8, 2016
@jvalkeal
Copy link
Collaborator Author

jvalkeal commented Apr 8, 2016

Right need to think this more for stream vs. tasks. Did some hacking locally with tasks:

dataflow:>task launch --name footask --arguments "--format=yyyy-MM-dd"

But arguments option as is doesn't really fit well with stream modules.

@sabbyanandan sabbyanandan changed the title Add option for command line args Add command line args support for tasks Apr 11, 2016
@cppwfs
Copy link
Contributor

cppwfs commented Apr 15, 2016

--params possibly (would match what we did for XD).

@jvalkeal
Copy link
Collaborator Author

Reason I went with --arguments is that java command as documented is:

java [ options ] -jar file.jar [ arguments ]

and this option would only add arguments and passing in -D, -X and other would not work.

@jvalkeal
Copy link
Collaborator Author

Anyway, I'm fine with --params if that's what we want. Additionally if we keep this only for tasks now and skip finding common ground for keeping things same(ish) for streams vs. task, it easy PR.

@mminella
Copy link
Contributor

Given ho vital this is for tasks, are there any objections to getting this in for the task side now and address streams later?

@jvalkeal
Copy link
Collaborator Author

Common ground was just brought in by @markpollack. As task is fundamentally different than stream, I personally don't see no problem having --params for tasks. With streams we'd just need to have a different format as you'd need to be able to pass unique params for all modules.

jvalkeal added a commit to jvalkeal/spring-cloud-dataflow that referenced this issue Apr 15, 2016
@jvalkeal
Add shell support for task params
99e52f8 
- 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
@jvalkeal jvalkeal mentioned this issue Apr 15, 2016
Closed
@jvalkeal jvalkeal added in pr and removed ready labels Apr 15, 2016
@ghillert ghillert mentioned this issue Apr 18, 2016
Closed
@jvalkeal
Copy link
Collaborator Author

I had some trouble to rebasing PR for this issue as there was bigger changes in tasks. We should probably do this post release as a new PR.

@sabbyanandan sabbyanandan removed this from the 1.0.0.M3 milestone Apr 26, 2016
@jvalkeal jvalkeal reopened this May 3, 2016
@jvalkeal jvalkeal added the ready label May 3, 2016
jvalkeal added a commit to jvalkeal/spring-cloud-dataflow that referenced this issue May 5, 2016
@jvalkeal
Add commandline params for tasks
40d18d3 
- Modify shell launch command.
- Add raw list of params throughout a chain
  from shell into a deployer SPI call.
- Added one test verifying params.
- In DefaultTaskJobService.restartJobExecution() params are
  coming from TaskExecution, theory at least, but this is not
  testable right now as we don't have restart command.
- Fixes spring-cloud#503
@jvalkeal jvalkeal mentioned this issue May 5, 2016
Closed
@jvalkeal jvalkeal added in pr and removed ready labels May 5, 2016
@sabbyanandan sabbyanandan mentioned this issue May 8, 2016
Closed
@sabbyanandan sabbyanandan added this to the 1.0.0.RC1 milestone May 8, 2016
jvalkeal added a commit to jvalkeal/spring-cloud-dataflow that referenced this issue May 16, 2016
@jvalkeal
Add commandline params for tasks
78d3468 
- Modify shell launch command.
- Add raw list of params throughout a chain
  from shell into a deployer SPI call.
- Added one test verifying params.
- In DefaultTaskJobService.restartJobExecution() params are
  coming from TaskExecution, theory at least, but this is not
  testable right now as we don't have restart command.
- Add parsing logic to DeploymentPropertiesUtils together
  with tests.
- Fixes spring-cloud#503
@jvalkeal jvalkeal mentioned this issue May 16, 2016
Closed
@mminella mminella removed the in pr label May 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jvalkeal jvalkeal

Labels
None yet
Projects
None yet
Milestone
1.0.0.RC1
Development

No branches or pull requests
4 participants
@jvalkeal @mminella @cppwfs @sabbyanandan