You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by Michael Osipov <mi...@apache.org> on 2019/10/22 13:09:16 UTC
Extending DefaultServiceUnavailableRetryStrategy
Friends,
while working on the PR [1] for WAGON-567, it came to my mind whether it
would be beneficial if
org.apache.http.impl.client.DefaultServiceUnavailableRetryStrategy would
accept a Collection<Integer> of status codes which can be retried.
Internally, that woudd be copied into a HashSet. If no values are
provided, we stick to SC_SERVICE_UNAVAILABLE.
WDYT?
Michael
[1] https://github.com/apache/maven-wagon/pull/57
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: Extending DefaultServiceUnavailableRetryStrategy
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Tue, 2019-10-22 at 15:09 +0200, Michael Osipov wrote:
> Friends,
>
> while working on the PR [1] for WAGON-567, it came to my mind whether
> it
> would be beneficial if
> org.apache.http.impl.client.DefaultServiceUnavailableRetryStrategy
> would
> accept a Collection<Integer> of status codes which can be retried.
>
> Internally, that woudd be copied into a HashSet. If no values are
> provided, we stick to SC_SERVICE_UNAVAILABLE.
>
> WDYT?
>
Sounds reasonable. Feel free to raise a PR with the proposed changes.
Cheers
Oleg
> Michael
>
> [1] https://github.com/apache/maven-wagon/pull/57
>
> ---------------------------------------------------------------------
> 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: Extending DefaultServiceUnavailableRetryStrategy
Posted by Gary Gregory <ga...@gmail.com>.
On Mon, Oct 28, 2019 at 7:10 AM Michael Osipov <mi...@apache.org> wrote:
> Am 2019-10-28 um 10:00 schrieb Oleg Kalnichevski:
> > On Sun, 2019-10-27 at 12:30 +0100, Michael Osipov wrote:
> >> Am 2019-10-23 um 09:35 schrieb Oleg Kalnichevski:
> >>> On Tue, 2019-10-22 at 19:47 +0200, Michael Osipov wrote:
> >>>> Am 2019-10-22 um 15:09 schrieb Michael Osipov:
> >>>>> Friends,
> >>>>>
> >>>>> while working on the PR [1] for WAGON-567, it came to my mind
> >>>>> whether it
> >>>>> would be beneficial if
> >>>>> org.apache.http.impl.client.DefaultServiceUnavailableRetryStrat
> >>>>> egy
> >>>>> would
> >>>>> accept a Collection<Integer> of status codes which can be
> >>>>> retried.
> >>>>>
> >>>>> Internally, that woudd be copied into a HashSet. If no values
> >>>>> are
> >>>>> provided, we stick to SC_SERVICE_UNAVAILABLE.
> >>>>
> >>>> To add some more fuel to the fire. Is
> >>>> ServiceUnavailableRetryStrategy
> >>>> intended for server-side errors only? I wonder why 429 is not
> >>>> included here.
> >>>>
> >>>
> >>> Yes, it was initially intended for recovery from temporary
> >>> unavailability of the target server.
> >>
> >> So from your POV, adding 4xx error would be an abuse of the
> >> interface?
> >> If so, what is the alternative for 408 or 429?
> >>
> >
> > No, it would not. ServiceUnavailableRetryStrategy was unfortunately not
> > the best choice of the name for the interface. It should have been
> > renamed in 5.0 but I feel it is a bit too late for that.
>
> Thanks for the clarification.
>
> We are still in beta, why not? Or make a generic interface RetryStrategy
> and make ServiceUnavailableRetryStrategy extend it with marked as
> deprecated. I was really confused by the name. According to it, the only
> viable status code is 503.
>
+1 to getting types properly named _now_ as names are critical IMO to
communicate intent. Being in Beta affords us this exact type of "luxury" ;-)
Gary
>
> Michael
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
>
Re: Extending DefaultServiceUnavailableRetryStrategy
Posted by Gary Gregory <ga...@gmail.com>.
On Mon, Oct 28, 2019 at 9:02 AM Oleg Kalnichevski <ol...@apache.org> wrote:
> On Mon, 2019-10-28 at 12:10 +0100, Michael Osipov wrote:
>
> ...
>
> > We are still in beta, why not? Or make a generic interface
> > RetryStrategy
> > and make ServiceUnavailableRetryStrategy extend it with marked as
> > deprecated. I was really confused by the name. According to it, the
> > only
> > viable status code is 503.
> >
> > Michael
> >
>
> Yes, we are technically still in BETA but the next release might be a
> GA. Anyway deprecation of ServiceUnavailableRetryStrategy in favor of a
> generic RetryStrategy sounds good like a good plan to me.
>
+1 to renaming. Deprecating one type for another while in Beta is, to me,
absurd. There is no cost to changing as the IDE does all the work.
Downstream call sites are using a Beta so know what they are in for: beta
code. I do not think one can -1 doing nothing but it looks bad to knowingly
leave cruft behind when not required to do so.
Gary
>
> Oleg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
>
Re: Extending DefaultServiceUnavailableRetryStrategy
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Mon, 2019-10-28 at 12:10 +0100, Michael Osipov wrote:
...
> We are still in beta, why not? Or make a generic interface
> RetryStrategy
> and make ServiceUnavailableRetryStrategy extend it with marked as
> deprecated. I was really confused by the name. According to it, the
> only
> viable status code is 503.
>
> Michael
>
Yes, we are technically still in BETA but the next release might be a
GA. Anyway deprecation of ServiceUnavailableRetryStrategy in favor of a
generic RetryStrategy sounds good like a good plan to me.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: Extending DefaultServiceUnavailableRetryStrategy
Posted by Michael Osipov <mi...@apache.org>.
Am 2019-10-28 um 10:00 schrieb Oleg Kalnichevski:
> On Sun, 2019-10-27 at 12:30 +0100, Michael Osipov wrote:
>> Am 2019-10-23 um 09:35 schrieb Oleg Kalnichevski:
>>> On Tue, 2019-10-22 at 19:47 +0200, Michael Osipov wrote:
>>>> Am 2019-10-22 um 15:09 schrieb Michael Osipov:
>>>>> Friends,
>>>>>
>>>>> while working on the PR [1] for WAGON-567, it came to my mind
>>>>> whether it
>>>>> would be beneficial if
>>>>> org.apache.http.impl.client.DefaultServiceUnavailableRetryStrat
>>>>> egy
>>>>> would
>>>>> accept a Collection<Integer> of status codes which can be
>>>>> retried.
>>>>>
>>>>> Internally, that woudd be copied into a HashSet. If no values
>>>>> are
>>>>> provided, we stick to SC_SERVICE_UNAVAILABLE.
>>>>
>>>> To add some more fuel to the fire. Is
>>>> ServiceUnavailableRetryStrategy
>>>> intended for server-side errors only? I wonder why 429 is not
>>>> included here.
>>>>
>>>
>>> Yes, it was initially intended for recovery from temporary
>>> unavailability of the target server.
>>
>> So from your POV, adding 4xx error would be an abuse of the
>> interface?
>> If so, what is the alternative for 408 or 429?
>>
>
> No, it would not. ServiceUnavailableRetryStrategy was unfortunately not
> the best choice of the name for the interface. It should have been
> renamed in 5.0 but I feel it is a bit too late for that.
Thanks for the clarification.
We are still in beta, why not? Or make a generic interface RetryStrategy
and make ServiceUnavailableRetryStrategy extend it with marked as
deprecated. I was really confused by the name. According to it, the only
viable status code is 503.
Michael
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: Extending DefaultServiceUnavailableRetryStrategy
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Sun, 2019-10-27 at 12:30 +0100, Michael Osipov wrote:
> Am 2019-10-23 um 09:35 schrieb Oleg Kalnichevski:
> > On Tue, 2019-10-22 at 19:47 +0200, Michael Osipov wrote:
> > > Am 2019-10-22 um 15:09 schrieb Michael Osipov:
> > > > Friends,
> > > >
> > > > while working on the PR [1] for WAGON-567, it came to my mind
> > > > whether it
> > > > would be beneficial if
> > > > org.apache.http.impl.client.DefaultServiceUnavailableRetryStrat
> > > > egy
> > > > would
> > > > accept a Collection<Integer> of status codes which can be
> > > > retried.
> > > >
> > > > Internally, that woudd be copied into a HashSet. If no values
> > > > are
> > > > provided, we stick to SC_SERVICE_UNAVAILABLE.
> > >
> > > To add some more fuel to the fire. Is
> > > ServiceUnavailableRetryStrategy
> > > intended for server-side errors only? I wonder why 429 is not
> > > included here.
> > >
> >
> > Yes, it was initially intended for recovery from temporary
> > unavailability of the target server.
>
> So from your POV, adding 4xx error would be an abuse of the
> interface?
> If so, what is the alternative for 408 or 429?
>
No, it would not. ServiceUnavailableRetryStrategy was unfortunately not
the best choice of the name for the interface. It should have been
renamed in 5.0 but I feel it is a bit too late for that.
Oleg
> Michael
>
> ---------------------------------------------------------------------
> 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: Extending DefaultServiceUnavailableRetryStrategy
Posted by Michael Osipov <mi...@apache.org>.
Am 2019-10-23 um 09:35 schrieb Oleg Kalnichevski:
> On Tue, 2019-10-22 at 19:47 +0200, Michael Osipov wrote:
>> Am 2019-10-22 um 15:09 schrieb Michael Osipov:
>>> Friends,
>>>
>>> while working on the PR [1] for WAGON-567, it came to my mind
>>> whether it
>>> would be beneficial if
>>> org.apache.http.impl.client.DefaultServiceUnavailableRetryStrategy
>>> would
>>> accept a Collection<Integer> of status codes which can be retried.
>>>
>>> Internally, that woudd be copied into a HashSet. If no values are
>>> provided, we stick to SC_SERVICE_UNAVAILABLE.
>>
>> To add some more fuel to the fire. Is
>> ServiceUnavailableRetryStrategy
>> intended for server-side errors only? I wonder why 429 is not
>> included here.
>>
>
> Yes, it was initially intended for recovery from temporary
> unavailability of the target server.
So from your POV, adding 4xx error would be an abuse of the interface?
If so, what is the alternative for 408 or 429?
Michael
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: Extending DefaultServiceUnavailableRetryStrategy
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Tue, 2019-10-22 at 19:47 +0200, Michael Osipov wrote:
> Am 2019-10-22 um 15:09 schrieb Michael Osipov:
> > Friends,
> >
> > while working on the PR [1] for WAGON-567, it came to my mind
> > whether it
> > would be beneficial if
> > org.apache.http.impl.client.DefaultServiceUnavailableRetryStrategy
> > would
> > accept a Collection<Integer> of status codes which can be retried.
> >
> > Internally, that woudd be copied into a HashSet. If no values are
> > provided, we stick to SC_SERVICE_UNAVAILABLE.
>
> To add some more fuel to the fire. Is
> ServiceUnavailableRetryStrategy
> intended for server-side errors only? I wonder why 429 is not
> included here.
>
Yes, it was initially intended for recovery from temporary
unavailability of the target server.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: Extending DefaultServiceUnavailableRetryStrategy
Posted by Michael Osipov <mi...@apache.org>.
Am 2019-10-22 um 15:09 schrieb Michael Osipov:
> Friends,
>
> while working on the PR [1] for WAGON-567, it came to my mind whether it
> would be beneficial if
> org.apache.http.impl.client.DefaultServiceUnavailableRetryStrategy would
> accept a Collection<Integer> of status codes which can be retried.
>
> Internally, that woudd be copied into a HashSet. If no values are
> provided, we stick to SC_SERVICE_UNAVAILABLE.
To add some more fuel to the fire. Is ServiceUnavailableRetryStrategy
intended for server-side errors only? I wonder why 429 is not included here.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org