Add allow_redirects to BaseSession
#174
Conversation
curl_cffi/requests/__init__.py
Outdated
| @@ -42,7 +42,7 @@ def request( | |||
| files: Optional[Dict] = None, | |||
| auth: Optional[Tuple[str, str]] = None, | |||
| timeout: Union[float, Tuple[float, float]] = 30, | |||
| allow_redirects: bool = True, | |||
| allow_redirects: bool = False, | |||
There was a problem hiding this comment.
It was kept as True to be consistent with requests.
There was a problem hiding this comment.
httpx reason:
Unlike requests, HTTPX does not follow redirects by default.
We differ in behaviour here because auto-redirects can easily mask unnecessary network calls being made.
There was a problem hiding this comment.
Also, I noticed curl-cffi uses infinity redirects (?)
Even in requests the default max_redirects is 30:
max_redirects
Maximum number of redirects allowed. If the request exceeds this limit, a TooManyRedirects exception is raised. This defaults to requests.models.DEFAULT_REDIRECT_LIMIT, which is 30.
There was a problem hiding this comment.
The ship has sailed, I don't think we should make a break change. However, we can reconsider this when planning 1.0
There was a problem hiding this comment.
Besides, it's set to True in aiohttp. For anyone using the AsyncSession and coming from that, it means less suprises.
There was a problem hiding this comment.
Yeah, max_redirects prabably should be changed.
There was a problem hiding this comment.
IIUC next release is 0.6.0?
IIRC, According to SEMVER specification, it's fine to include breaking changes between minor releases in 0.x
btw, if you think deprecation process must apply here, I'm also OK with that ;)
There was a problem hiding this comment.
For anyone using the AsyncSession and coming from that, it means less suprises
I'm usually use httpx in python, at first when I used curl-cffi, I suprised that I should use allow_redirects=False for every s.get|post(). because redirects can cause unwanted behaviors.
btw, allowing to configure it in AsyncSession class can help me on this. I'm fine to use allow_redirects=False one time on session initializing.
|
@yifeikong Can you review now? |
No description provided.