Hi Andreas, First and foremost, I'm not asking to revert the patch as well. You have been a great collaborator and an active contributor to many parts of the code through out the past, and even right now. What I wanted here is to find a way to keep the existing API as it is, and also introduce your change.
On Fri, Aug 21, 2009 at 12:40 AM, Andreas Veithen <andreas.veit...@gmail.com > wrote: > On Thu, Aug 20, 2009 at 18:08, Senaka Fernando<sen...@wso2.com> wrote: > > Hi Andreas, > > > > Just wondering what you are trying to achieve here. > > Fixing AxisServlet so that it works as expected, i.e. deliver the > functionalities that the existing code promised to implement, but > failed to deliver. Great. > > > > Is this related to auto > > detection of ports as Hiranya pointed out? > > Yes. Note that port autodetection has always been part of the code, > but it was deeply buried in the code. It is now done at the right > level, i.e. at the same level where we do autodetection of the context > path. Understood. But, is port auto-detection something that should happen by default? Also, what if someone extends the AxisServlet (I see you have pointed out the fact below). > > > > While I appreciate the effort > > you've put into doing something worthwhile, I believe that getting rid of > a > > public method in a class is not the correct thing to do. > > As explained earlier, Sorry, I missed this. > it is not the purpose of the fix to get rid of > this public method and this will be undone. Great. > > > > I believe that what > > you have done here is the addition of a new portion of code. Can we make > the > > new portion of code optional? > > Since it is supposed to replace pieces of code that didn't work as > expected, there is no point in making them optional. It's either > accepting the change or design a different fix that solves these > issues. Well, I've got some questions here. Port auto-detection perhaps never worked properly, well, making it optional here means that, if someone wants auto-detection by default, he can have it, but if someone doesn't, he should not. I guess we'll need to figure out how we could achieve this in the initialization phase. > > > > And leave the existing logic as it was? > > The existing logic is responsible for 6 JIRAs and complaints on the > mailing list. Hmm... well, I don't quite see you getting rid of the existing logic in your commit e-mail. But, noticed the new enhancements you made. I'm referring to the AxisServlet class only in here. > > > > Also, > > are you planning further changes to this class? > > We need to make sure that SynapseAxisServlet can safely extend > AxisServlet without tight coupling to the internals of AxisServlet. > This will probably require some (hopefully non breaking) changes in > AxisServlet. Yes please. This should answer several questions that I'm having right now regarding this prevalent API (after your commit). > > > > if so, it would perhaps be > > better to figure out a more elaborate solution, which safeguards both the > > existing level of extensibility of this class and also its public API. > > The problem is that AxisServlet is not designed to be extensible. That > is the fundamental problem we need to find a solution for. Any > suggestions are welcome. Agreed. I'll have a look at the code once again, and get back to you. But, it is wrong to say that the class is not designed to be extensible, IMHO. To a certain extent it was, but with the introduction of private methods, it has reduced significantly. However, what I'm trying to do here is not asking to revert your hard work to resolve 6 issues that solves a tough problem. What I'd like is to see how it fits in our code with minimum impact to the former API. Thanks, Senaka > > > > > > Thanks, > > Senaka > > > > On Thu, Aug 20, 2009 at 6:14 PM, Hiranya Jayathilaka < > hiranya...@gmail.com> > > wrote: > >> > >> Hi Andreas, > >> > >> By looking at the code I got the impression that HTTP transport > receivers > >> should extend the AxisServletListener class for your logic of port auto > >> detection to work. Is that correct? What happens if the transport > receivers > >> used do not extend this class? All request handler methods call the > >> preprocessRequest method which in turns run port auto detection. If the > >> transport receivers do not extend AxisServlerListener how is that > handled? > >> > >> Thanks, > >> Hiranya > >> > >> > >> On Thu, Aug 20, 2009 at 6:05 PM, Andreas Veithen > >> <andreas.veit...@gmail.com> wrote: > >>> > >>> Afkham, > >>> > >>> The only change I see in the public APIs is the disappearance of the > >>> initContextRoot method. We can easily fix this be restoring the > >>> original initContextRoot method and let the preprocessRequest method > >>> call initContextRoot. Do you see any other things to change? > >>> > >>> Andreas > >>> > >>> On Thu, Aug 20, 2009 at 13:45, Afkham Azeez<afk...@gmail.com> wrote: > >>> > Yes Dims. However, if everybody continues to merrily change APIs, > >>> > making public methods private & so on, things are going to become a > >>> > big mess. Axis2 provides public APIs, and those may be having > >>> > problems, but still they are public APIs. This is why you have to be > >>> > very careful when defining APIs; if you get them wrong, you may have > >>> > to live with it for a long time. > >>> > > >>> > Azeez > >>> > > >>> > On Thu, Aug 20, 2009 at 11:38 AM, Davanum Srinivas<dava...@gmail.com > > > >>> > wrote: > >>> >> Azeez, > >>> >> > >>> >> We are still following, commit-then-review right? > >>> >> > >>> >> thanks, > >>> >> dims > >>> >> > >>> >> On 08/20/2009 07:33 AM, Afkham Azeez wrote: > >>> >>> > >>> >>> Hi Andreas, > >>> >>> The changes you've done to the APIs as per > >>> >>> https://issues.apache.org/jira/browse/AXIS2-4465 badly breaks some > of > >>> >>> the projects that depend on Axis2. Please revert this, and please > >>> >>> engage the community before making such drastic changes in the > >>> >>> future. > >>> >>> > >>> >> > >>> > > >>> > > >>> > > >>> > -- > >>> > Thanks > >>> > Afkham Azeez > >>> > > >>> > Blog: http://afkham.org > >>> > Developer Portal: http://www.wso2.org > >>> > WSAS Blog: http://wso2wsas.blogspot.com > >>> > Company: http://wso2.com > >>> > GPG Fingerprint: 643F C2AF EB78 F886 40C9 B2A2 4AE2 C887 665E 0760 > >>> > > >> > >> > >> > >> -- > >> Hiranya Jayathilaka > >> Software Engineer; > >> WSO2 Inc.; http://wso2.org > >> E-mail: hira...@wso2.com; Mobile: +94 77 633 3491 > >> Blog: http://techfeast-hiranya.blogspot.com > > > > >
