You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by sebb <se...@gmail.com> on 2014/02/25 20:07:16 UTC
Re: svn commit: r1571715 - /httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/AsynchronousValidationRequest.java
On 25 February 2014 14:46, <ol...@apache.org> wrote:
> Author: olegk
> Date: Tue Feb 25 14:46:07 2014
> New Revision: 1571715
>
> URL: http://svn.apache.org/r1571715
> Log:
> Made AsynchronousValidationRequest public
Should this be applied to 4.3.x as well?
I'm happy to do it if so.
> Modified:
> httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/AsynchronousValidationRequest.java
>
> Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/AsynchronousValidationRequest.java
> URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/AsynchronousValidationRequest.java?rev=1571715&r1=1571714&r2=1571715&view=diff
> ==============================================================================
> --- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/AsynchronousValidationRequest.java (original)
> +++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/AsynchronousValidationRequest.java Tue Feb 25 14:46:07 2014
> @@ -45,7 +45,7 @@ import org.apache.http.conn.routing.Http
> * Class used to represent an asynchronous revalidation event, such as with
> * "stale-while-revalidate"
> */
> -class AsynchronousValidationRequest implements Runnable {
> +public class AsynchronousValidationRequest implements Runnable {
> private final AsynchronousValidator parent;
> private final CachingExec cachingExec;
> private final HttpRoute route;
> @@ -108,7 +108,7 @@ class AsynchronousValidationRequest impl
> * @return <code>true</code> if the cache entry was successfully validated;
> * otherwise <code>false</code>
> */
> - protected boolean revalidateCacheEntry() {
> + private boolean revalidateCacheEntry() {
> try {
> final CloseableHttpResponse httpResponse = cachingExec.revalidateCacheEntry(route, request, context, execAware, cacheEntry);
> try {
> @@ -164,7 +164,7 @@ class AsynchronousValidationRequest impl
> return true;
> }
>
> - String getIdentifier() {
> + public String getIdentifier() {
> return identifier;
> }
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1571715
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Mon, 2014-03-03 at 18:57 +0000, sebb wrote:
> On 3 March 2014 10:43, Oleg Kalnichevski <ol...@apache.org> wrote:
> > On Sun, 2014-03-02 at 01:24 +0000, sebb wrote:
...
> >>
> >> I don't know which of its fields are likely to be needed by the
> >> schedule() method apart from consecutiveFailedAttempts which already
> >> has a public getter.
> >> The trunk version now has a public getter for the identifier; for
> >> consistency perhaps that should also be added to 4.3.x?
> >>
> >>
> >
> > HttpClient 4.3 ships with two public implementations of this interface.
> > Those classes are likely to be enough for the majority of users. I would
> > rather not make any API changes to the stable branch unless absolutely
> > necessary.
>
> In which case, I think the Javadoc needs to be updated to note this.
>
> However, I just don't understand the rigid stance on API immutability.
> Yes, changes should be carefully reviewed to ensure that they make sense.
> But in this case the next release has already made the proposed change.
> And backwards compatibility is very unlikely to be an issue.
>
We still have a chance to re-do those changes in the dev branch should
we find that necessary. We will not have that option anymore once 4.3.4
is out.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1571715
Posted by sebb <se...@gmail.com>.
On 3 March 2014 10:43, Oleg Kalnichevski <ol...@apache.org> wrote:
> On Sun, 2014-03-02 at 01:24 +0000, sebb wrote:
>> On 26 February 2014 14:03, Oleg Kalnichevski <ol...@apache.org> wrote:
>> > On Tue, 2014-02-25 at 19:07 +0000, sebb wrote:
>> >> On 25 February 2014 14:46, <ol...@apache.org> wrote:
>> >> > Author: olegk
>> >> > Date: Tue Feb 25 14:46:07 2014
>> >> > New Revision: 1571715
>> >> >
>> >> > URL: http://svn.apache.org/r1571715
>> >> > Log:
>> >> > Made AsynchronousValidationRequest public
>> >>
>> >> Should this be applied to 4.3.x as well?
>> >>
>> >> I'm happy to do it if so.
>> >>
>> >
>> > I am not sure. I generally avoid adding new classes and methods in
>> > stable branches. I would just leave 4.3.x as it is, but we will not
>> > stand in your way if you decide otherwise.
>>
>> AFAICT it's not possible at present for anyone to create a
>> SchedulingStrategy unless the class is in the same package as
>> AsynchronousValidationRequest.
>>
>> It's not possible to implement the required interface method:
>>
>> void schedule(AsynchronousValidationRequest revalidationRequest);
>>
>> without having access to the AsynchronousValidationRequest class which
>> is currently package protected.
>>
>> So I think the class must be made public unless there is no intention
>> to allow users to provide their own SchedulingStrategy.
>>
>> I don't know which of its fields are likely to be needed by the
>> schedule() method apart from consecutiveFailedAttempts which already
>> has a public getter.
>> The trunk version now has a public getter for the identifier; for
>> consistency perhaps that should also be added to 4.3.x?
>>
>>
>
> HttpClient 4.3 ships with two public implementations of this interface.
> Those classes are likely to be enough for the majority of users. I would
> rather not make any API changes to the stable branch unless absolutely
> necessary.
In which case, I think the Javadoc needs to be updated to note this.
However, I just don't understand the rigid stance on API immutability.
Yes, changes should be carefully reviewed to ensure that they make sense.
But in this case the next release has already made the proposed change.
And backwards compatibility is very unlikely to be an issue.
> Oleg
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1571715
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Sun, 2014-03-02 at 01:24 +0000, sebb wrote:
> On 26 February 2014 14:03, Oleg Kalnichevski <ol...@apache.org> wrote:
> > On Tue, 2014-02-25 at 19:07 +0000, sebb wrote:
> >> On 25 February 2014 14:46, <ol...@apache.org> wrote:
> >> > Author: olegk
> >> > Date: Tue Feb 25 14:46:07 2014
> >> > New Revision: 1571715
> >> >
> >> > URL: http://svn.apache.org/r1571715
> >> > Log:
> >> > Made AsynchronousValidationRequest public
> >>
> >> Should this be applied to 4.3.x as well?
> >>
> >> I'm happy to do it if so.
> >>
> >
> > I am not sure. I generally avoid adding new classes and methods in
> > stable branches. I would just leave 4.3.x as it is, but we will not
> > stand in your way if you decide otherwise.
>
> AFAICT it's not possible at present for anyone to create a
> SchedulingStrategy unless the class is in the same package as
> AsynchronousValidationRequest.
>
> It's not possible to implement the required interface method:
>
> void schedule(AsynchronousValidationRequest revalidationRequest);
>
> without having access to the AsynchronousValidationRequest class which
> is currently package protected.
>
> So I think the class must be made public unless there is no intention
> to allow users to provide their own SchedulingStrategy.
>
> I don't know which of its fields are likely to be needed by the
> schedule() method apart from consecutiveFailedAttempts which already
> has a public getter.
> The trunk version now has a public getter for the identifier; for
> consistency perhaps that should also be added to 4.3.x?
>
>
HttpClient 4.3 ships with two public implementations of this interface.
Those classes are likely to be enough for the majority of users. I would
rather not make any API changes to the stable branch unless absolutely
necessary.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1571715
Posted by sebb <se...@gmail.com>.
On 26 February 2014 14:03, Oleg Kalnichevski <ol...@apache.org> wrote:
> On Tue, 2014-02-25 at 19:07 +0000, sebb wrote:
>> On 25 February 2014 14:46, <ol...@apache.org> wrote:
>> > Author: olegk
>> > Date: Tue Feb 25 14:46:07 2014
>> > New Revision: 1571715
>> >
>> > URL: http://svn.apache.org/r1571715
>> > Log:
>> > Made AsynchronousValidationRequest public
>>
>> Should this be applied to 4.3.x as well?
>>
>> I'm happy to do it if so.
>>
>
> I am not sure. I generally avoid adding new classes and methods in
> stable branches. I would just leave 4.3.x as it is, but we will not
> stand in your way if you decide otherwise.
AFAICT it's not possible at present for anyone to create a
SchedulingStrategy unless the class is in the same package as
AsynchronousValidationRequest.
It's not possible to implement the required interface method:
void schedule(AsynchronousValidationRequest revalidationRequest);
without having access to the AsynchronousValidationRequest class which
is currently package protected.
So I think the class must be made public unless there is no intention
to allow users to provide their own SchedulingStrategy.
I don't know which of its fields are likely to be needed by the
schedule() method apart from consecutiveFailedAttempts which already
has a public getter.
The trunk version now has a public getter for the identifier; for
consistency perhaps that should also be added to 4.3.x?
> Oleg
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1571715
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Tue, 2014-02-25 at 19:07 +0000, sebb wrote:
> On 25 February 2014 14:46, <ol...@apache.org> wrote:
> > Author: olegk
> > Date: Tue Feb 25 14:46:07 2014
> > New Revision: 1571715
> >
> > URL: http://svn.apache.org/r1571715
> > Log:
> > Made AsynchronousValidationRequest public
>
> Should this be applied to 4.3.x as well?
>
> I'm happy to do it if so.
>
I am not sure. I generally avoid adding new classes and methods in
stable branches. I would just leave 4.3.x as it is, but we will not
stand in your way if you decide otherwise.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org