You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@continuum.apache.org by Brett Porter <br...@apache.org> on 2008/05/02 06:53:04 UTC
Re: refactoring the SCM
me too after cleaning up. Sorry about that, I'll look into it.
- Brett
On 30/04/2008, at 5:52 AM, Olivier Lamy wrote:
> Hi,
> I can't build the branch :
> /local/olamy/open-source/continuum-svn/builder-branch/continuum-core/
> src/main/java/org/apache/maven/continuum/core/action/
> AddProjectToCheckOutQueueAction.java:[60,16]
> cannot find symbol
> symbol : class CheckOutTask
> location: class
> org.apache.maven.continuum.core.action.AddProjectToCheckOutQueueAction
>
>
> 2008/4/27 Brett Porter <br...@apache.org>:
>> Hi,
>>
>> I've started to do some refactoring - this is along the way to the
>> builder
>> separation I mentioned earlier.
>>
>> If you have a moment, please review r 651947. It's on a branch, but
>> I'd
>> like to regularly merge to trunk if there are no objects to avoid
>> getting
>> too distant. I have more tests to write for this first.
>>
>> All I've done is pulled the DefaultContinuumScm class out into a
>> separate
>> module, and decoupled it from the model. It already contained some
>> logic
>> related to the SCMs (which may actually need to go back into Maven
>> SCM
>> itself). I pulled the logging and database updates back into the
>> actions in
>> code (this did result in some duplication, but I can clean that up
>> later).
>> It also showed that some code was never having its results used,
>> and also
>> started to expose some exception handling bugs. I stopped wrapping
>> exceptions and results, choosing to use the Maven SCM API natively.
>>
>> Thoughts?
>>
>> Anyone that is knowledgable in Spring, please check my work :) Is
>> there a
>> way to easily populate maps of beans, instead of hard coding the
>> providers?
>>
>
> hehe it looks plexus have some nice features ;-)
>
>> Cheers,
>> Brett
>>
>> --
>> Brett Porter
>> brett@apache.org
>> http://blogs.exist.com/bporter/
>>
>>
--
Brett Porter
brett@apache.org
http://blogs.exist.com/bporter/
Re: refactoring next steps (was: refactoring the SCM)
Posted by Brett Porter <br...@apache.org>.
On 07/05/2008, at 6:29 AM, Emmanuel Venisse wrote:
> Hi Brett,
>
> Great work, I like it and I'm ok to merge it regularly to trunk.
Cool. I'll give it another day and a bit more of a spin then do that.
>
>
> I don't have idea about maps of beans.
>
> I think this branch is a good start to do more refactoring.
> 1- split DefaultBuildController to a controller + a builder
> The builder should be in it's own module and doesn't use/know the
> store, it
> checkout/update the working copy and build the project
agreed - this is one of the key objectives of the structure I brought
up before.
>
> The controller get an event from the UI, the scheduler or an other
> way and
> send and event to one or more builder (local or remote), JMS is
> probably the
> solution
yeah, I think the first stage is just to make sure the code is
separate enough that you could replace the implementation.
I think JMS might only be needed for the events - I like the idea of a
RESTful request model to the separate builder.
>
> 2- for each steps, the builder send the new project state
> notification, by
> JMS too and the Controller store it in memory, The project state
> must be
> transient instead of to store it in the db like today.
By state, do you mean "building", "checking out", "svn error" (which
are transient), or "success", "fail" (which are permanent and
historical?). I agree on the first ones - actually Marica mentioned
this in a previous mail and said she was going to start a new thread
for it :)
>
> When the build is done, the builder send a new event with the build
> result
Yep - we can probably send progress events too.
>
>
> I have a point that I don't know for the moment how to resolve. How to
> access to the working copy when the builder is a remote builder and
> how to
> do when we use more than one remote builder? But we'll can find a
> solution
> later.
well, I don't think the working copy belongs under the main interface
at that point - you should be able to go straight to the remote server
and access it over HTTP (we'll still need to wire in security, but
that's needed for any RESTful interface to them too). The aggregating
interface can probably link out to them or integrate the listing using
javascript or something.
Cheers,
Brett
--
Brett Porter
brett@apache.org
http://blogs.exist.com/bporter/
Re: refactoring the SCM
Posted by Emmanuel Venisse <em...@gmail.com>.
Hi Brett,
Great work, I like it and I'm ok to merge it regularly to trunk.
I don't have idea about maps of beans.
I think this branch is a good start to do more refactoring.
1- split DefaultBuildController to a controller + a builder
The builder should be in it's own module and doesn't use/know the store, it
checkout/update the working copy and build the project
The controller get an event from the UI, the scheduler or an other way and
send and event to one or more builder (local or remote), JMS is probably the
solution
2- for each steps, the builder send the new project state notification, by
JMS too and the Controller store it in memory, The project state must be
transient instead of to store it in the db like today.
When the build is done, the builder send a new event with the build result
I have a point that I don't know for the moment how to resolve. How to
access to the working copy when the builder is a remote builder and how to
do when we use more than one remote builder? But we'll can find a solution
later.
Emmanuel
On Tue, May 6, 2008 at 3:37 AM, Brett Porter <br...@apache.org> wrote:
> ok, fixed and committed :)
>
> Anyone able to take a look through?
>
> Cheers,
> Brett
>
>
> On 02/05/2008, at 2:53 PM, Brett Porter wrote:
>
> me too after cleaning up. Sorry about that, I'll look into it.
> >
> > - Brett
> >
> > On 30/04/2008, at 5:52 AM, Olivier Lamy wrote:
> >
> > Hi,
> > > I can't build the branch :
> > >
> > > /local/olamy/open-source/continuum-svn/builder-branch/continuum-core/src/main/java/org/apache/maven/continuum/core/action/AddProjectToCheckOutQueueAction.java:[60,16]
> > > cannot find symbol
> > > symbol : class CheckOutTask
> > > location: class
> > > org.apache.maven.continuum.core.action.AddProjectToCheckOutQueueAction
> > >
> > >
> > > 2008/4/27 Brett Porter <br...@apache.org>:
> > >
> > > > Hi,
> > > >
> > > > I've started to do some refactoring - this is along the way to the
> > > > builder
> > > > separation I mentioned earlier.
> > > >
> > > > If you have a moment, please review r 651947. It's on a branch, but
> > > > I'd
> > > > like to regularly merge to trunk if there are no objects to avoid
> > > > getting
> > > > too distant. I have more tests to write for this first.
> > > >
> > > > All I've done is pulled the DefaultContinuumScm class out into a
> > > > separate
> > > > module, and decoupled it from the model. It already contained some
> > > > logic
> > > > related to the SCMs (which may actually need to go back into Maven
> > > > SCM
> > > > itself). I pulled the logging and database updates back into the
> > > > actions in
> > > > code (this did result in some duplication, but I can clean that up
> > > > later).
> > > > It also showed that some code was never having its results used, and
> > > > also
> > > > started to expose some exception handling bugs. I stopped wrapping
> > > > exceptions and results, choosing to use the Maven SCM API natively.
> > > >
> > > > Thoughts?
> > > >
> > > > Anyone that is knowledgable in Spring, please check my work :) Is
> > > > there a
> > > > way to easily populate maps of beans, instead of hard coding the
> > > > providers?
> > > >
> > > >
> > > hehe it looks plexus have some nice features ;-)
> > >
> > > Cheers,
> > > > Brett
> > > >
> > > > --
> > > > Brett Porter
> > > > brett@apache.org
> > > > http://blogs.exist.com/bporter/
> > > >
> > > >
> > > >
> > --
> > Brett Porter
> > brett@apache.org
> > http://blogs.exist.com/bporter/
> >
> >
> --
> Brett Porter
> brett@apache.org
> http://blogs.exist.com/bporter/
>
>
Re: refactoring the SCM
Posted by Brett Porter <br...@apache.org>.
Ah... actually from memory I think they all throw ScmException, the
others are there for "more information", but afaik they won't impact
the contract.
- Brett
On 07/05/2008, at 11:19 AM, Rahul Thakur wrote:
>
> The motivation behind this was that layer/module should (ideally)
> expose limited number of granular exception(s). Having said that, I
> agree wrapping-unwrapping exceptions can get clunky and contract
> definition should be merited by the exception handling instances
> that we have observed in the code.
>
> I wasn't referring to nesting exceptions (may be wrong use of
> 'wrap'), but unifying the hierarchy of ScmExceptions, so clients can
> introspect using 'instanceof' to determine specific ones.
>
> Rahul
>
>
> Brett Porter wrote:
>>
>> On 07/05/2008, at 10:27 AM, Rahul Thakur wrote:
>>
>>>
>>> Cool! :)
>>>
>>> Just one note on exceptions - Can we wrap up all the SCM exceptions
>>> under one parent which is then exposed through the ContinuumScm API?
>>>
>>> Clients that need to do any special handling can introspect the
>>> extension.
>>>
>>> WDYT?
>>
>> One of the problems in the code that was just removed was that some
>> exceptions were getting swallowed or handled in the wrong place
>> because
>> it was trying to introspect a nested exception.
>>
>> I don't really think wrapping an exception without adding any
>> information, only to unwrap it later makes much sense.
>>
>> I think the caller should be able to deal with the Maven SCM
>> exceptions,
>> and the IOException resulting from passing a bad working directory.
>>
>> On the other hand, we don't really want to change interfaces in the
>> future, and a wrapped exception does provide that flexibility like
>> the
>> request parameter does.
>>
>> What do others think?
>>
>> - Brett
>>
>> --
>> Brett Porter
>> brett@apache.org
>> http://blogs.exist.com/bporter/
>>
>>
--
Brett Porter
brett@apache.org
http://blogs.exist.com/bporter/
Re: refactoring the SCM
Posted by Rahul Thakur <ra...@gmail.com>.
The motivation behind this was that layer/module should (ideally) expose
limited number of granular exception(s). Having said that, I agree
wrapping-unwrapping exceptions can get clunky and contract definition
should be merited by the exception handling instances that we have
observed in the code.
I wasn't referring to nesting exceptions (may be wrong use of 'wrap'),
but unifying the hierarchy of ScmExceptions, so clients can introspect
using 'instanceof' to determine specific ones.
Rahul
Brett Porter wrote:
>
> On 07/05/2008, at 10:27 AM, Rahul Thakur wrote:
>
>>
>> Cool! :)
>>
>> Just one note on exceptions - Can we wrap up all the SCM exceptions
>> under one parent which is then exposed through the ContinuumScm API?
>>
>> Clients that need to do any special handling can introspect the
>> extension.
>>
>> WDYT?
>
> One of the problems in the code that was just removed was that some
> exceptions were getting swallowed or handled in the wrong place because
> it was trying to introspect a nested exception.
>
> I don't really think wrapping an exception without adding any
> information, only to unwrap it later makes much sense.
>
> I think the caller should be able to deal with the Maven SCM exceptions,
> and the IOException resulting from passing a bad working directory.
>
> On the other hand, we don't really want to change interfaces in the
> future, and a wrapped exception does provide that flexibility like the
> request parameter does.
>
> What do others think?
>
> - Brett
>
> --
> Brett Porter
> brett@apache.org
> http://blogs.exist.com/bporter/
>
>
Re: refactoring the SCM
Posted by Brett Porter <br...@apache.org>.
On 07/05/2008, at 10:27 AM, Rahul Thakur wrote:
>
> Cool! :)
>
> Just one note on exceptions - Can we wrap up all the SCM exceptions
> under one parent which is then exposed through the ContinuumScm API?
>
> Clients that need to do any special handling can introspect the
> extension.
>
> WDYT?
One of the problems in the code that was just removed was that some
exceptions were getting swallowed or handled in the wrong place
because it was trying to introspect a nested exception.
I don't really think wrapping an exception without adding any
information, only to unwrap it later makes much sense.
I think the caller should be able to deal with the Maven SCM
exceptions, and the IOException resulting from passing a bad working
directory.
On the other hand, we don't really want to change interfaces in the
future, and a wrapped exception does provide that flexibility like the
request parameter does.
What do others think?
- Brett
--
Brett Porter
brett@apache.org
http://blogs.exist.com/bporter/
Re: refactoring the SCM
Posted by Rahul Thakur <ra...@gmail.com>.
Cool! :)
Just one note on exceptions - Can we wrap up all the SCM exceptions
under one parent which is then exposed through the ContinuumScm API?
Clients that need to do any special handling can introspect the extension.
WDYT?
Cheers,
Rahul
Brett Porter wrote:
> ok, fixed and committed :)
>
> Anyone able to take a look through?
>
> Cheers,
> Brett
>
> On 02/05/2008, at 2:53 PM, Brett Porter wrote:
>
>> me too after cleaning up. Sorry about that, I'll look into it.
>>
>> - Brett
>>
>> On 30/04/2008, at 5:52 AM, Olivier Lamy wrote:
>>
>>> Hi,
>>> I can't build the branch :
>>> /local/olamy/open-source/continuum-svn/builder-branch/continuum-core/src/main/java/org/apache/maven/continuum/core/action/AddProjectToCheckOutQueueAction.java:[60,16]
>>>
>>> cannot find symbol
>>> symbol : class CheckOutTask
>>> location: class
>>> org.apache.maven.continuum.core.action.AddProjectToCheckOutQueueAction
>>>
>>>
>>> 2008/4/27 Brett Porter <br...@apache.org>:
>>>> Hi,
>>>>
>>>> I've started to do some refactoring - this is along the way to the
>>>> builder
>>>> separation I mentioned earlier.
>>>>
>>>> If you have a moment, please review r 651947. It's on a branch, but I'd
>>>> like to regularly merge to trunk if there are no objects to avoid
>>>> getting
>>>> too distant. I have more tests to write for this first.
>>>>
>>>> All I've done is pulled the DefaultContinuumScm class out into a
>>>> separate
>>>> module, and decoupled it from the model. It already contained some
>>>> logic
>>>> related to the SCMs (which may actually need to go back into Maven SCM
>>>> itself). I pulled the logging and database updates back into the
>>>> actions in
>>>> code (this did result in some duplication, but I can clean that up
>>>> later).
>>>> It also showed that some code was never having its results used, and
>>>> also
>>>> started to expose some exception handling bugs. I stopped wrapping
>>>> exceptions and results, choosing to use the Maven SCM API natively.
>>>>
>>>> Thoughts?
>>>>
>>>> Anyone that is knowledgable in Spring, please check my work :) Is
>>>> there a
>>>> way to easily populate maps of beans, instead of hard coding the
>>>> providers?
>>>>
>>>
>>> hehe it looks plexus have some nice features ;-)
>>>
>>>> Cheers,
>>>> Brett
>>>>
>>>> --
>>>> Brett Porter
>>>> brett@apache.org
>>>> http://blogs.exist.com/bporter/
>>>>
>>>>
>>
>> --
>> Brett Porter
>> brett@apache.org
>> http://blogs.exist.com/bporter/
>>
>
> --
> Brett Porter
> brett@apache.org
> http://blogs.exist.com/bporter/
>
>
Re: refactoring the SCM
Posted by Olivier Lamy <ol...@apache.org>.
Hi,
Looks fine.
Thanks,
--
Olivier
2008/5/6 Brett Porter <br...@apache.org>:
> ok, fixed and committed :)
>
> Anyone able to take a look through?
>
> Cheers,
> Brett
>
>
>
> On 02/05/2008, at 2:53 PM, Brett Porter wrote:
>
>
> > me too after cleaning up. Sorry about that, I'll look into it.
> >
> > - Brett
> >
> > On 30/04/2008, at 5:52 AM, Olivier Lamy wrote:
> >
> >
> > > Hi,
> > > I can't build the branch :
> > >
> /local/olamy/open-source/continuum-svn/builder-branch/continuum-core/src/main/java/org/apache/maven/continuum/core/action/AddProjectToCheckOutQueueAction.java:[60,16]
> > > cannot find symbol
> > > symbol : class CheckOutTask
> > > location: class
> > > org.apache.maven.continuum.core.action.AddProjectToCheckOutQueueAction
> > >
> > >
> > > 2008/4/27 Brett Porter <br...@apache.org>:
> > >
> > > > Hi,
> > > >
> > > > I've started to do some refactoring - this is along the way to the
> builder
> > > > separation I mentioned earlier.
> > > >
> > > > If you have a moment, please review r 651947. It's on a branch, but
> I'd
> > > > like to regularly merge to trunk if there are no objects to avoid
> getting
> > > > too distant. I have more tests to write for this first.
> > > >
> > > > All I've done is pulled the DefaultContinuumScm class out into a
> separate
> > > > module, and decoupled it from the model. It already contained some
> logic
> > > > related to the SCMs (which may actually need to go back into Maven SCM
> > > > itself). I pulled the logging and database updates back into the
> actions in
> > > > code (this did result in some duplication, but I can clean that up
> later).
> > > > It also showed that some code was never having its results used, and
> also
> > > > started to expose some exception handling bugs. I stopped wrapping
> > > > exceptions and results, choosing to use the Maven SCM API natively.
> > > >
> > > > Thoughts?
> > > >
> > > > Anyone that is knowledgable in Spring, please check my work :) Is
> there a
> > > > way to easily populate maps of beans, instead of hard coding the
> providers?
> > > >
> > > >
> > >
> > > hehe it looks plexus have some nice features ;-)
> > >
> > >
> > > > Cheers,
> > > > Brett
> > > >
> > > > --
> > > > Brett Porter
> > > > brett@apache.org
> > > > http://blogs.exist.com/bporter/
> > > >
> > > >
> > > >
> > >
> >
> > --
> > Brett Porter
> > brett@apache.org
> > http://blogs.exist.com/bporter/
> >
> >
>
> --
> Brett Porter
> brett@apache.org
> http://blogs.exist.com/bporter/
>
>
Re: refactoring the SCM
Posted by Brett Porter <br...@apache.org>.
ok, fixed and committed :)
Anyone able to take a look through?
Cheers,
Brett
On 02/05/2008, at 2:53 PM, Brett Porter wrote:
> me too after cleaning up. Sorry about that, I'll look into it.
>
> - Brett
>
> On 30/04/2008, at 5:52 AM, Olivier Lamy wrote:
>
>> Hi,
>> I can't build the branch :
>> /local/olamy/open-source/continuum-svn/builder-branch/continuum-
>> core/src/main/java/org/apache/maven/continuum/core/action/
>> AddProjectToCheckOutQueueAction.java:[60,16]
>> cannot find symbol
>> symbol : class CheckOutTask
>> location: class
>> org
>> .apache.maven.continuum.core.action.AddProjectToCheckOutQueueAction
>>
>>
>> 2008/4/27 Brett Porter <br...@apache.org>:
>>> Hi,
>>>
>>> I've started to do some refactoring - this is along the way to the
>>> builder
>>> separation I mentioned earlier.
>>>
>>> If you have a moment, please review r 651947. It's on a branch,
>>> but I'd
>>> like to regularly merge to trunk if there are no objects to avoid
>>> getting
>>> too distant. I have more tests to write for this first.
>>>
>>> All I've done is pulled the DefaultContinuumScm class out into a
>>> separate
>>> module, and decoupled it from the model. It already contained some
>>> logic
>>> related to the SCMs (which may actually need to go back into Maven
>>> SCM
>>> itself). I pulled the logging and database updates back into the
>>> actions in
>>> code (this did result in some duplication, but I can clean that up
>>> later).
>>> It also showed that some code was never having its results used,
>>> and also
>>> started to expose some exception handling bugs. I stopped wrapping
>>> exceptions and results, choosing to use the Maven SCM API natively.
>>>
>>> Thoughts?
>>>
>>> Anyone that is knowledgable in Spring, please check my work :) Is
>>> there a
>>> way to easily populate maps of beans, instead of hard coding the
>>> providers?
>>>
>>
>> hehe it looks plexus have some nice features ;-)
>>
>>> Cheers,
>>> Brett
>>>
>>> --
>>> Brett Porter
>>> brett@apache.org
>>> http://blogs.exist.com/bporter/
>>>
>>>
>
> --
> Brett Porter
> brett@apache.org
> http://blogs.exist.com/bporter/
>
--
Brett Porter
brett@apache.org
http://blogs.exist.com/bporter/