You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-dev@axis.apache.org by Glen Daniels <gl...@thoughtcraft.com> on 2007/04/25 22:26:13 UTC
[axis2] Recent changes
Hi folks:
Please consider yourself encouraged to review my last couple of commits,
which include two significant items.
1) I am in the midst of cleaning up and simplifying the deployment
system a bit.
2) I removed the InstanceDispatcher, since it really wasn't a Dispatcher
at all, but rather a place for functionality (setting up contexts) that
should always happen at the end of dispatching - sometimes it was in the
Dispatch phase, and sometimes in the PostDispatch phase. I moved that
functionality into DispatchPhase.checkPostConditions(), so now it's
built in to the end of the phase.
Also, now that <handler>s that are configured in axis2.xml inside a
<phase> tag automatically know what Phase they're in from the
containment relationship, I removed a lot of the unnecessary <order
phase=""> tags from most of the config files in our source tree.
Please let me know if you have any comments. Thanks!
--Glen
---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org
Re: [axis2] Recent changes
Posted by David Illsley <da...@gmail.com>.
I've just realised you did leave the class there. Sorry for the
unnecessary advice.
David
On 26/04/07, David Illsley <da...@gmail.com> wrote:
> I totally agree that the InstanceDispatcher is ill-named and should
> either be renamed or refactored into the DispatchPhase. I also agree
> that it can't simply be removed. I suggest putting the logic wherever
> is more logical and leaving the InstanceDispatcher class there, marked
> as deprecated which calls the logic wherever you put it. We can then
> put in the release notes that it is deprecated and remove the class
> after the next release.
> David
>
--
David Illsley - IBM Web Services Development
---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org
Re: [axis2] Recent changes
Posted by Sanjiva Weerawarana <sa...@opensource.lk>.
s/InstanceDispatcher/SessionDispatcher/ ?
I'm +1 for keeping it and not removing it.
Sanjiva.
David Illsley wrote:
> I totally agree that the InstanceDispatcher is ill-named and should
> either be renamed or refactored into the DispatchPhase. I also agree
> that it can't simply be removed. I suggest putting the logic wherever
> is more logical and leaving the InstanceDispatcher class there, marked
> as deprecated which calls the logic wherever you put it. We can then
> put in the release notes that it is deprecated and remove the class
> after the next release.
> David
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
> For additional commands, e-mail: axis-dev-help@ws.apache.org
>
>
--
Sanjiva Weerawarana, Ph.D.
Founder & Director; Lanka Software Foundation; http://www.opensource.lk/
Founder, Chairman & CEO; WSO2, Inc.; http://www.wso2.com/
Director; Open Source Initiative; http://www.opensource.org/
Member; Apache Software Foundation; http://www.apache.org/
Visiting Lecturer; University of Moratuwa; http://www.cse.mrt.ac.lk/
---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org
Re: [axis2] Recent changes
Posted by David Illsley <da...@gmail.com>.
I totally agree that the InstanceDispatcher is ill-named and should
either be renamed or refactored into the DispatchPhase. I also agree
that it can't simply be removed. I suggest putting the logic wherever
is more logical and leaving the InstanceDispatcher class there, marked
as deprecated which calls the logic wherever you put it. We can then
put in the release notes that it is deprecated and remove the class
after the next release.
David
---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org
Re: [axis2] Recent changes
Posted by Deepal Jayasinghe <de...@opensource.lk>.
Hi Glen
Glen Daniels wrote:
>
> The reason I removed it was because sometimes it was configured in the
> "PostDispatch" phase and sometimes it was configured in the "Dispatch"
> phase - if it ran PostDispatch, as it was in some stuff I was testing
> with, the checks in the DispatchPhase post-conditions didn't work in
> some situations because the contexts were not set up correctly.
Well , we do not have phase called PostDispatch any more , global flow
will finish once it reaches Dispatch phase.
>
> Before I roll the changes back, let me see if I can convince you to
> rescind your -1....
I will keep my -1 as it is until I go through the code VERY carefully
and make sure you have not break any major features. Session management
had several issues and I fixed most of them b4 1.1 release. So I need to
make sure when moving code from ID to post dispatch phase you have not
miss any thing.
I will go through the code immediately after I am done with 1.2 release.
>
> First of all, there is no backward compatibility breakage. All the
> functionality of the InstanceDispatcher still exists - the
> loadContext() method has simply moved to DispatchPhase so that it
> happens automatically at the end of EVERY dispatch phase regardless of
> how people want to configure their dispatchers.
Well , I see a big issue here .
Let's say someone need to put handler after InstanceDispatcher to
validate some property available in the contexts (smt like user
validation), and if smt goes wrong drop the message at that place. But
when we move InstanceDispatcher code into post condition of the Dispatch
phase , he will no longer be able to do that. So that is the backward
compatibility issue I am talking about.
I personally think keeping description dispatching and context
dispatching in two separate (in a configurable place) places more value
than moving code in to non-configurable place like post condition of
Dispatch phase.
> And as David mentioned, I've left the class there so even old
> axis2.xml's with references to InstanceDispatcher will continue to
> work without any problem. If we want to remove the class for good at
> some point, that's the only time breakage will occur, and we can warn
> people well in advance. I certainly don't think anyone is relying on
> the InstanceDispatcher being a configurable Handler, that's just the
> way we had it working.
>
> Regarding Dispatchers, they exist so that we can get the correct
> AxisService and AxisOperation in a /variety/ of ways. The reason this
> functionality isn't built in to the system at a lower level (i.e. why
> we use Handlers) is so you can tweak your axis2.xml and decide only to
> have the URL based dispatch, for instance, or only have one custom
> dispatcher. The context setup seems to me to be a different kind of
> thing - once you've got the AxisService/AxisOperation.
Agreed.
>
> I would also really like to complete a review on the design of the way
> ServiceContext/SessionContext/ServiceGroupContext are related (we
> discussed this at ApacheCon last year), and that connects to this code
> as well....
Do you see any issues with that ?,
As I can see only issue is lack of documents , not the relationship of
the context , so let's try to solve that problem first and then we will
see the issue.
>
> So what I'd like to do now is this - leave the change there (since it
> won't break anyone), and finish reviewing the context relationships.
> Once that's done I'll email the list with a proposal for how I'd like
> to clean things up, and how that will affect this code.
+1
Thanks
Deepal
---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org
Re: [axis2] Recent changes
Posted by Glen Daniels <gl...@thoughtcraft.com>.
Hi Deepal!
Deepal Jayasinghe wrote:
>> 2) I removed the InstanceDispatcher, since it really wasn't a
>> Dispatcher at all,
>
> Why not it dispatch contexts , so it is a Dispatcher.
See below.
>> but rather a place for functionality (setting up contexts) that should
>> always happen at the end of dispatching - sometimes it was in the
>> Dispatch phase, and sometimes in the PostDispatch phase. I moved that
>> functionality into DispatchPhase.checkPostConditions(), so now it's
>> built in to the end of the phase.
>>
> I am big -1 on this change , doing such a major changes after stables
> releases is not a good idea at all, as well as that will break backward
> compatibility among the releases. Btw was there a particular reason for
> removing InstanceDispatcher ?.
The reason I removed it was because sometimes it was configured in the
"PostDispatch" phase and sometimes it was configured in the "Dispatch"
phase - if it ran PostDispatch, as it was in some stuff I was testing
with, the checks in the DispatchPhase post-conditions didn't work in
some situations because the contexts were not set up correctly. So
rather than having this be something a configuration screwup can break,
I decided to relocate the same functionality - now the context setup
occurs someplace which will always be called, just like the check for
valid operation/service.
Before I roll the changes back, let me see if I can convince you to
rescind your -1....
First of all, there is no backward compatibility breakage. All the
functionality of the InstanceDispatcher still exists - the loadContext()
method has simply moved to DispatchPhase so that it happens
automatically at the end of EVERY dispatch phase regardless of how
people want to configure their dispatchers. And as David mentioned,
I've left the class there so even old axis2.xml's with references to
InstanceDispatcher will continue to work without any problem. If we
want to remove the class for good at some point, that's the only time
breakage will occur, and we can warn people well in advance. I
certainly don't think anyone is relying on the InstanceDispatcher being
a configurable Handler, that's just the way we had it working.
Regarding Dispatchers, they exist so that we can get the correct
AxisService and AxisOperation in a /variety/ of ways. The reason this
functionality isn't built in to the system at a lower level (i.e. why we
use Handlers) is so you can tweak your axis2.xml and decide only to have
the URL based dispatch, for instance, or only have one custom
dispatcher. The context setup seems to me to be a different kind of
thing - once you've got the AxisService/AxisOperation.
I would also really like to complete a review on the design of the way
ServiceContext/SessionContext/ServiceGroupContext are related (we
discussed this at ApacheCon last year), and that connects to this code
as well....
So what I'd like to do now is this - leave the change there (since it
won't break anyone), and finish reviewing the context relationships.
Once that's done I'll email the list with a proposal for how I'd like to
clean things up, and how that will affect this code. Then we can have a
discussion to finalize how we want it to work, and I'll change it to
match whatever we resolve at that point.
Let me know what you think of this line of reasoning, and if you still
-1 it, I can move the logic back before I make the other proposal.
Thanks,
--Glen
---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org
Re: [axis2] Recent changes
Posted by Deepal Jayasinghe <de...@opensource.lk>.
Hi Glen
> Hi folks:
>
> Please consider yourself encouraged to review my last couple of
> commits, which include two significant items.
>
> 1) I am in the midst of cleaning up and simplifying the deployment
> system a bit.
cool.
>
> 2) I removed the InstanceDispatcher, since it really wasn't a
> Dispatcher at all,
Why not it dispatch contexts , so it is a Dispatcher.
> but rather a place for functionality (setting up contexts) that should
> always happen at the end of dispatching - sometimes it was in the
> Dispatch phase, and sometimes in the PostDispatch phase. I moved that
> functionality into DispatchPhase.checkPostConditions(), so now it's
> built in to the end of the phase.
>
I am big -1 on this change , doing such a major changes after stables
releases is not a good idea at all, as well as that will break backward
compatibility among the releases. Btw was there a particular reason for
removing InstanceDispatcher ?.
I am ok with improving logic in th InstanceDispatcher , but please do
not remove that .
> Also, now that <handler>s that are configured in axis2.xml inside a
> <phase> tag automatically know what Phase they're in from the
> containment relationship, I removed a lot of the unnecessary <order
> phase=""> tags from most of the config files in our source tree.
+1
>
> Please let me know if you have any comments. Thanks!
>
> --Glen
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
> For additional commands, e-mail: axis-dev-help@ws.apache.org
>
>
>
"
---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org