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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to