Allow zero max retries

[poweroftrue]

Hello @HoneyryderChuck,

Your work is a piece of art.

I'm not sure if there's a better way to make httpx zero retires, I tried faraday.request :retry, max: 0but it didn't work.

This came up while I was using the click_house gem: shlima/click_house#49

[HoneyryderChuck]

Hi @poweroftrue thx for the patch.

If I understood your issue correctly, you're having trouble with the faraday adapter, which loafs the persistent plugin, which indirectly loads the.retry plugin with max_retries set to 1.

First, faraday retries are not.built on top of httpx.retries,so you won't fix that with this patch.

Second, while it's arguable that max_retries set to 0 makes sense, doing so would break the persistent plugin which requires a retry to recover from lost connections.

What problem are you trying to fix exactly.in this case?

[poweroftrue]

Hello @HoneyryderChuck

If I understood your issue correctly, you're having trouble with the faraday adapter, which loafs the persistent plugin, which indirectly loads the.retry plugin with max_retries set to 1.

Exactly, I want to disable the retries because I'm doing a POST to non-idempotent endpoint, retires even on failure or timeout are very risky, and causes duplications.

While retries are good but for me use case I want zero retries.

This patch actually worked with me, if there's an alternative way to disable retries, let me know.

[HoneyryderChuck]

As per docs, by default, request on non-idempotent won't be retried (you can enable that by setting retry_change_requests option to true.

[poweroftrue]

@HoneyryderChuck I just checked, seems like even if the query changes data, the request is GET and still will make duplicates if retried.

I would understand if you don't want to support zero retries for GET requests, but I think many will prefer to handle the retry logic at their end.

[HoneyryderChuck]

seems like even if the query changes data, the request is GET and still will make duplicates if retried.

You mentioned in an earlier message that "I'm doing a POST to non-idempotent endpoint", but are now mentioning a GET? I'm a bit confused, which one of them you do not want to retry? (As mentioned, by default it won't retry on POST, if it does that would be a bug).

I would understand if you don't want to support zero retries for GET requests, but I think many will prefer to handle the retry logic at their end.

I'm not saying (yet) that it does not make sense, but as it stands, as mentioned earlier, it'd degrade the persistent plugin if done so, and overall I don't see the benefit against the option of not loading the retries plugin. You have many options to tune when not to retry (retry_change_requests as mentioned for non-idempotent requests, but there's also the retry_on callback if you want to deny/allow certain urls or types of errors). What I would like to understand is why none of these options work for your use-case, ideally with a reproduction of the issue.

[poweroftrue]

You mentioned in an earlier message that "I'm doing a POST to non-idempotent endpoint", but are now mentioning a GET? I'm a bit confused, which one of them you do not want to retry? (As mentioned, by default it won't retry on POST, if it does that would be a bug).

I thought it was POST I checked it when you mentioned this point, it's a GET request but it writes some data, retries duplicate the data.

I'm not saying (yet) that it does not make sense, but as it stands, as mentioned earlier, it'd degrade the persistent plugin if done so, and overall I don't see the benefit against the option of not loading the retries plugin. You have many options to tune when not to retry (retry_change_requests as mentioned for non-idempotent requests, but there's also the retry_on callback if you want to deny/allow certain urls or types of errors). What I would like to understand is why none of these options work for your use-case, ideally with a reproduction of the issue.

My use case is using Farady I have GET requests that should never be retried, I want to disable retries completely, I tried using faraday way to disable retries still HTTPx retires requests.

One more issue related to this is when the user configure retries on top of Faraday/HTTPx it end up sending a lot more requests than expected (if he doesn't know about the default retry behavior).

[HoneyryderChuck]

I thought it was POST I checked it when you mentioned this point, it's a GET request

Is this an API endpoint you can control, and change into a POST? IMO this is just wrong interpretation of the meaning of HTTP verbs, which has wider repercussions than just the retry behaviour you're trying to patch in httpx (p.ex. if your requests are proxied, the several middlewares may themselves retry the request if it's an idempotent GET).

My use case is using Farady I have GET requests that should never be retried

I see. I've had past annoyances reported about faraday defaults before. For example, some clients which are less tolerant to open connections, had to patch the faraday adapter with the "persistent: false" option in order to disable the persistent behaviour. And now for your use case, you'd like to "disable" retries. I believe this is a symptom of a larger problem, which are the faraday httpx adapter defaults, and how some users really don't need them.

I'll have to think about this a bit more, as even if I'd consider removing them, I'll need a migration path, and perhaps what you are proposing. Plus, km on holidays, and that's take a bit one way or another.

[poweroftrue]

Is this an API endpoint you can control, and change into a POST? IMO this is just wrong interpretation of the meaning of HTTP verbs, which has wider repercussions than just the retry behaviour you're trying to patch in httpx (p.ex. if your requests are proxied, the several middlewares may themselves retry the request if it's an idempotent GET).

That's true, I will try to get that changed internally.

I see. I've had past annoyances reported about faraday defaults before. For example, some clients which are less tolerant to open connections, had to patch the faraday adapter with the "persistent: false" option in order to disable the persistent behaviour. And now for your use case, you'd like to "disable" retries. I believe this is a symptom of a larger problem, which are the faraday httpx adapter defaults, and how some users really don't need them.
I'll have to think about this a bit more, as even if I'd consider removing them, I'll need a migration path, and perhaps what you are proposing. Plus, km on holidays, and that's take a bit one way or another.

I'm still new to Ruby, but tell me what you have in mind I will PR something and you give it a look, what do you think?

Feel free to close this PR it was just to get the conversation going and to quickly patch my thing

[HoneyryderChuck]

Tbf if you can change the endpoint into a POST, that's the quickest way to patch your issue. I won't be back to programming until the week after the next, and I'll need to think about the root cause and a fix. Thx for trying a patch though, and I'll keep it open until I consider whether it makes sense or not in the context of faraday integration.

[HoneyryderChuck]

cherry-picked here. I can't think of a better way to handle this right now, so this should unblock you, with the mentioned caveats.