tuscany-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Raymond Feng <enjoyj...@gmail.com>
Subject Re: svn commit: r1303591 - /tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
Date Thu, 22 Mar 2012 16:38:09 GMT
Ant, please STOP playing the "veto" game here.

1. Your change broke most if not all the binding invokers that make
outbound connections using the binding uri even though the failures were
not directly exposed due to lack of test coverage which we should improve.

2. The Endpoint concept was introduced later in the cycle. As a result,
most of the reference binding invokers still depend on the behavior that
the reference binding is set with the deployed service endpoint uri. We can
argue if it's good or bad but we need to fix them before another bug fix
regressed the code so much. I will try to fix the binding invokers but it
will take a bit time.

3. I have the same veto right too. The only thing is that my revert didn't
completely roll back all your changes including the failing compliance test
if it's added recently. I explained in the e-mail why I need to revert your
change. Maybe I should say -1 or veto your change too to make it clear.

4. Please don't abuse/stain the Apache hat when/if you try to push for some
'private' agenda.

Sorry for being harsh here but otherwise we'll be painted as anti-Apache if
we have technical disagreement.

Thanks,
Raymond

On Thu, Mar 22, 2012 at 7:50 AM, ant elder <ant.elder@gmail.com> wrote:

> On Thu, Mar 22, 2012 at 2:46 PM, Luciano Resende <luckbr1975@gmail.com>
> wrote:
> >
> >
> > On Thursday, March 22, 2012, ant elder <ant.elder@gmail.com> wrote:
> >> On Wed, Mar 21, 2012 at 9:49 PM,  <rfeng@apache.org> wrote:
> >>> Author: rfeng
> >>> Date: Wed Mar 21 21:49:52 2012
> >>> New Revision: 1303591
> >>>
> >>> URL: http://svn.apache.org/viewvc?rev=1303591&view=rev
> >>> Log:
> >>> Revert the change based on the comment from
> >>> https://issues.apache.org/jira/browse/TUSCANY-4029
> >>>
> >>> Modified:
> >>>
> >>>
>  tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
> >>>
> >>> Modified:
> >>>
> tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
> >>> URL:
> >>>
> http://svn.apache.org/viewvc/tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java?rev=1303591&r1=1303590&r2=1303591&view=diff
> >>>
> >>>
> ==============================================================================
> >>> ---
> >>>
> tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
> >>> (original)
> >>> +++
> >>>
> tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
> >>> Wed Mar 21 21:49:52 2012
> >>> @@ -338,6 +338,11 @@ public class EndpointReferenceBinderImpl
> >>>
> >>>         }
> >>>
> >>> +        // [rfeng] Setup the target endpoint if the reference uses an
> >>> explicit binding
> >>> +        if (endpointReference.getTargetEndpoint().getBinding() ==
> null)
> >>> {
> >>> +
> >>>
>  endpointReference.getTargetEndpoint().setBinding(endpointReference.getBinding());
> >>> +        }
> >>> +
> >>>         // Now the endpoint reference is resolved check that the
> binding
> >>> interfaces contract
> >>>         // and the reference contract are compatible
> >>>         try {
> >>> @@ -500,12 +505,16 @@ public class EndpointReferenceBinderImpl
> >>>         } else {
> >>>             endpointReference.setTargetEndpoint(matchedEndpoint);
> >>>             Binding binding = matchedEndpoint.getBinding();
> >>> +            // Reverted the change, see
> >>> https://issues.apache.org/jira/browse/TUSCANY-4029
> >>> +            /*
> >>>             try {
> >>>                                endpointReference.setBinding((Binding)
> >>> binding.clone());
> >>>                        } catch (CloneNotSupportedException e) {
> >>>                                // shouldn't happen
> >>>                                throw new RuntimeException(e);
> >>>                        }
> >>> +          */
> >>> +            endpointReference.setBinding(binding);
> >>>             // TUSCANY-3873 - add policy from the service
> >>>             //                we don't care about intents at this stage
> >>>
> >>>
> endpointReference.getPolicySets().addAll(matchedEndpoint.getPolicySets());
> >>> @@ -528,6 +537,7 @@ public class EndpointReferenceBinderImpl
> >>>
> >>>
> endpointReference.setStatus(EndpointReference.Status.WIRED_TARGET_FOUND_AND_MATCHED);
> >>>          Raymond, as discussed in TUSCANY-4029 and on the IM chat we
> had
> >> yesterday i'm -1 on your commit. It doesn't fix the problem which
> >> TUSCANY-4029 addresses and it also breaks the OASIS compliance tests.
> >>
> >> If there are any issues with the approach suggested in TUSCANY-4029
> >> then lets discuss it here or you're welcome to IM ping me to more
> >> quickly find a solution that works for everyone.
> >>
> >>   ...ant
> >>
> >
> > Please, let's keep these technical discussions on the mailing list.
> >
> > BTW, how come it breakes the compliance test ? It has been like this
> forever
> > and the tests were passing fine.
> >
> > In the same way, your fix seems to break other stuff as well as
> commented on
> > the other thread, which I'm -1 on having all that broken.
> >
>
> Can you provide more details about what other stuff is broken? The
> build works, does the update suggested in TUSCANY-4029 fix the problem
> with the rest invoker?
>
>   ...ant
>

Mime
View raw message