[
https://issues.apache.org/jira/browse/SLIDER-780?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14522850#comment-14522850
]
thomas liu commented on SLIDER-780:
-----------------------------------
Thank you for your comments!
Now I'm still working on the fun-tests of this patch.
Given we need to install Docker on the target hosts, writing unit test with
minicluster is a little tricker
{quote}
I think we can get rid of variable con and declare controller at global level.
Then call controller.actionQueue.dockerManager.stop_container() in
signal_handler.
{quote}
Agree. Also seen your comment below regarding the failing test case.
{quote}
Is there a reason to use the digit 2 in the method names? If not, please rename
them appropriately.
{quote}
No there isn't...It was a result of previous temporary "addStartCommand2" code
in the private branches. I just blindly merged them in. Will rename them.
{quote}
return type.toLowerCase().equals("docker");
Let's define string constant for "docker" in SliderUtils.java just like PYTHON
is defined.
cmdParams.put("script_type", "PYTHON");
Let's define TYPE_PYTHON in AbstractComponent.java just like you have
TYPE_DOCKER.
{quote}
Agree.
{quote}
Not required now, but at some point we need to define PythonExecutionCommand
and DockerExecutionCommand to modularize the code. This will also pave the way
for supporting additional executors/containers in the future, like Flocker
(which is picking some steam as well).
{quote}
Agree. Also have this in my radar. Will simplify the params we pass to Agent as
well. Besides, PythonExecutionCommand and DockerExecutionCommand, there can be
ShellCommandExecutionCommand as well.
{quote}
Use formatter {} instead of string concat.
{quote}
Agree. Those are old logging code. Will change their level.
{quote}
Seems like we can use entrySet() as you are accessing key and value.
{quote}
Agree
{quote}
DockerContainerPort.java, DockerContainerMount.java, and DockerContainer.java
public void validate(String version) throws SliderException {
Add @Override to all validate methods.
{quote}
Agree
{quote}
Use StringBuilder
{quote}
Agree
{quote}
Some generic formatting comments:
{quote}
Sounds good
Thank you for your comments!
Will post my new patch & with test cases tomorrow.
> Support for Docker based application packaging in Slider
> --------------------------------------------------------
>
> Key: SLIDER-780
> URL: https://issues.apache.org/jira/browse/SLIDER-780
> Project: Slider
> Issue Type: New Feature
> Reporter: thomas liu
> Assignee: thomas liu
> Fix For: Slider 0.80
>
> Attachments: JIRA-780-001.patch, SlidersupportingDockersummary (1).pdf
>
>
> Enable Slider to deploy an application defined as Docker image, monitor its
> running status, fetching exported configs, and maintain its lifecycle.
> Please find the details of this ticket in the attachment
> 'SlidersupportingDockersummary(1).pdf'
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
