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

Already on GitHub? Sign in to your account

ARQ-708 Should support keeping container alive between execution phases #2

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
3 participants
Member

DavideD commented Dec 22, 2011

Hi,
here's my first prototype.

Cheers

Member

DavideD commented Jan 20, 2012

Right now the plugin has a goal that starts the container and deploys the archive.
Would it make sense to create two separate goals for this so that you can start a conatiner without deploy anything in it?
Same can be apply when stopping the container.

Owner

aslakknutsen commented Jan 23, 2012

Yes, i think this would make sense. You could then use the Arq Plugin just to Start and have other means of deploying.

Member

DavideD commented Jan 23, 2012

I've split start, deploy, undeploy and stop.

What do you think?

Owner

aslakknutsen commented Jan 24, 2012

Why are all the start/stop logic in BaseCommand and not in Start/StopCommand ?

Member

DavideD commented Jan 24, 2012

Sleep deprivation :)

I think I've fixed the problem but I'm not sure how to manage the manager. RIght now it's closed when the server is stopped and when there is an exception.

Owner

aslakknutsen commented Jan 25, 2012

nice!

one little thing, we currently have to use both start and deploy commands on command line when we want to deploy to a remote container. This doesn't make much sense on a managed, but start/connect is the same in the underlying container impl.

mvn arquillian:start mvn arquillian:deploy

Previously deploy would also start. Does it make sense for deploy to start if no manager found in context? Similar with undeploy as well. And if deploy/undeploy does start the manager, they also do stop

?

Member

DavideD commented Jan 28, 2012

I think it make sense to start the container before deploy and undeploy if the start has not been executed before.

I would stop the container as last operation if it's not been stopped before but I don't know how to do that in the maven plugin at the moment.

Member

DavideD commented Jan 28, 2012

Deploy and Undeploy are now starting the container if it's not started before. Seems to work fine.
I added a profile with a jbossas 7 managed container to experiment with it.

I'm still not sure when to stop the container.

This is the tricky case: deploy, undeploy.

deploy shouldn't stop the container because there is another command and undeploy doesn't know if there is a stop command after that so when I'm performing the task I'm not sure If I have to stop it.

Member

DavideD commented Jan 28, 2012

What about adding one last command: "undeployAndStop"?

Member

DavideD commented Jan 28, 2012

or the stop could undeploy if something has been deployed (and not undeployed) before.

mvn arquilliian:deploy arquillian:stop

instead of

mvn arquillian:deploy arquillian:undeploy

Is it counter-intuitive having a stop command that undeploys when needed?

Owner

aslakknutsen commented Feb 1, 2012

How about this:

arquillian:start

  • calls container start and shares manager in context

arquillian:deploy

  • if manager in context
    • calls deploy
  • if no manager in context
    • calls start, deploy, stop

arquillian:undeploy

  • if manager in context
    • calls undeploy
  • if no manager in context
  • calls start, undeploy, stop

arquillian:stop

  • calls container stop and removes manager from context

arquillian:run

  • ignore manager in context
  • calls start, deploy (... holds until ctrl-c ...), undeploy, stop

Deploy and Undeploy alone only makes sense for Remote containers where Start/Stop act as connect/disconnect.
Run can work with any Container type but meant as a 'quick view' option.
Start and Stop (combined or not with Deploy/Undeploy) can work on anything where you want more fine grained control over which Maven lifecycle should execute which Container cylce

?

Member

DavideD commented Feb 1, 2012

I think the only reserve I have with this approach is when someone calls only deploy and undeploy in a managed environment because between the two calls it will restart the container. Am I correct?

I suppose that in that case it make sense to use start/stop and we can warn the user about it.

Owner

aslakknutsen commented Feb 1, 2012

correct, in a managed/embedded scenario you would use "run" for quick preview or bind both cycles.

We have 3 main usecases i think:

  • command line: preview app
    For this you use arquillian:run which work with all container types
  • command line: deploy / undeploy to a running server
    For this you can use the 'new' arquillian:deploy and arquillian:undeploy logic, where start/stop only work as connect/disconnect
  • automated testing start/stop/deploy/undeploy
    For this you would bind the plugin to the Maven lifecycles as you see fit.
Owner

aslakknutsen commented Feb 1, 2012

we also possible have:

  • automated: deploy / undeploy to a running server
    For this you can use arquillian:deploy and arquillian:undeploy or bind all cycles to the Maven lifecycle
Member

DavideD commented Feb 1, 2012

Ok, I see now. I will fix this this evening.

Member

DavideD commented Feb 1, 2012

I've applied the fix. The only difference is the deploy. I cannot stop the container after the deploy because I want to do something between the deploy and undeploy phases the container will be stopped (in the embedded and managed situation at least).

I've tried to add a ShutdownHook so that if nobody Stop the container it will be eventually stop at the end of the build. I'm not sure if this is correct.

Owner

aslakknutsen commented Feb 2, 2012

If you want to do something between deploy and undeploy, then you should define start/deploy.. do something, undeploy/stop..

only defining deploy or undeploy is only intended as a 'short cut' for the commandline ?

Owner

aslakknutsen commented Feb 2, 2012

maybe its to magical what is suppose to happen..

maybe we should simply add two new commands:

deployRemote and undeployRemote

where both of them do start/stop before and after and ignore the shared context manager if any. Then we have deploy/undeploy require start to be called first and stop after

Member

DavideD commented Feb 2, 2012

What about an attribute instead of two new commands?
Something like

<configuration>
<remoteMode>true</remoteMode>
</configuration>

???
By default is true. This way if you add start and stop in the pom it doesn't really matter if it's managed or remote.

Member

DavideD commented Feb 2, 2012

I suppose there is no easy way to check if the container in the classpath is remote/manged/embedded?

Owner

aslakknutsen commented Feb 3, 2012

no, remote/managed/embedded is hidden

Owner

aslakknutsen commented Feb 3, 2012

not sure i quite get the remoteMode. if true, deploy would start and stop? while if false it will not stop?

Member

DavideD commented Feb 3, 2012

  1. remoteMode = true

deploy/undeploy start and stop the server after every execution.

  1. remoteMode = false
    If deploy/undeploy without start is called I can throw an exception and warn the user.

The benefit is that if someone uses start, deploy, undeploy, stop in the pom it can switch between the two mode just by setting a property. and not by changing the commands. From the commmand line , deploy and undeploy can be used without problems.

I cannot think of any way to check that at some point the stop is called before the end of execution.
By the way, what are the consequences of not calling stop? When I use it I don't see any difference. After all at the end of the maven build the container is stopped anyway.

Owner

aslakknutsen commented Feb 3, 2012

It will 'stop' when the jvm stop, but it's a forced kill, not a clean shutdown.

Member

DavideD commented Feb 3, 2012

I see....
Another benefit of the properties is that if at some point there is a way to understand if we are in remote/managed/embedded we can actually ignore it and the commands are not gonna change.

Would it make sense to add a method to the DeployableContainer taht says the type of the container (remote/embedded/managed)?

Owner

aslakknutsen commented Feb 3, 2012

But i don't understand the undeploy and stop problem. undeploy should only call stop if it called start. if start is configured, we'll have to assume the user has configured stop as well.

Member

DavideD commented Feb 3, 2012

Yes, it's fine for me,.I was just trying to figure out if there is a way to validate the case when the user use start but he doesn't use the stop.

Anyway, I hink that to add deployRemote/undeployRemote is the best solution. It make it more clear the separation between the two set of commands.

So in the end we will have:

  1. deployRemote/undeployRemote that can be used without calling Start and Stop (they will start and stop the server anyway)
  2. deploy/undeploy that should be used with Start/Stop: they will throw an exception/warning if Start is not called

This is what you suggested in the beginning, isn't it?

Owner

aslakknutsen commented Feb 3, 2012

it was suggested midway or so yes.. :)

sure, let's do that. We're still Alpha and can always revisit if needed :)

Member

DavideD commented Feb 4, 2012

This should work.

Cheers

Owner

bartoszmajsak commented Feb 4, 2012

Hi guys,

from my point of view having clear separation between remote / standalone & embedded is the way to go. I would rather like to use start + deploy and deployRemote instead of having some magic trick which actually makes real meaning of deploy quite fuzzy.

Great stuff by the way! Nice to see an alternative to Cargo :)

Member

DavideD commented Feb 4, 2012

Thanks for the feedback!
This version should work exactly like that.

Cheers

Owner

aslakknutsen commented Feb 6, 2012

Awesome! Davide, you're the man!

Squashed and pushed upstream: f3d459b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment