remove requests dependency by glyph · Pull Request #401 · twisted/treq

glyph · 2024-09-19T19:57:05Z

Fixes #325

In order to know whether we are even testing the various requests extensions, we want to have type hints that actually reflect the signatures on all the helpers.

I've definitely bitten off more than I can chew for getting this done today, so if anyone else wants to have a look at this, please be my guest.

… to get a correct type error

twm · 2024-09-21T05:24:24Z

    return _client(kwargs).head(url, _stacklevel=4, **kwargs)


+@_like(HTTPClient.get)


Interesting approach... I have a branch somewhere where I started factoring out TypedDict for all the arguments but it is so painful I haven't worked up the will to finish it.

Oh, huh, will Mypy actually do the right thing with a TypedDict in this context?

(I do think I prefer this approach regardless, that one had just never occurred to me.)

I expect it will given Unpack (my experience actually doing this is with Pyright).

I certainly agree that this is less painful!

It turns out that there are details that this can't capture (the addition of the agent= argument) so I ended up skipping it.

in the course of fixing a bunch of conflicts, also add a bunch of types

glyph · 2026-05-30T23:07:52Z

2 things in particular that I'd like feedback on from a reviewer, beyond the usual:

This ended up pulling in a lot of typing-related fixes because I didn't want to make this change without some type hints giving me at least some indication that I was properly passing everything down.
TreqieJar is obviously a terrible name, but I didn't want to drop indexing support on the result type because that would be a hard compatibility break. any thoughts?
what kind of doc updates would be best for this

adiroiban

Is the goal of this PR to have a complete removal of requests ?

I guess that we should also update Python package definition and remove the requests dependency.

treq/pyproject.toml

Line 40 in 724e458

"requests >= 2.1.0",

Regarding typing. I think that it's a good improvement.
I don't have much feedback about typing as I am still not using Python typing on my day to day work... so I don't have much experience with that.

I don't think this needs more docs.

I think that the current docs page is good enough

https://treq.readthedocs.io/en/latest/#why

while on it, I have update the treq repo to link to the docs since I guess that this is the public website for the project

glyph · 2026-05-31T22:28:39Z

Is the goal of this PR to have a complete removal of requests ?

I guess that we should also update Python package definition and remove the requests dependency.

oh, yeah, this definitely needs to be done.

glyph · 2026-05-31T23:19:40Z

oh, yeah, this definitely needs to be done.

aaaand done, in edd757f . I manually reviewed some pip freeze output in various tox environments to make sure that it is, indeed, not installed.

adiroiban

It looks good.

Maybe we can have a CI task to check that 'requests' is not a dependency.

As a start. Something simple.
Maybe add this before installing tox.

treq/.github/workflows/ci.yaml

Lines 54 to 56 in 724e458

    
                     ${{ runner.os }}-pip- 
        
               - run: python -m pip install 'tox<4'

    - name: Check that requests is not installed
      run: |
        python -m pip install .
        if python -m pip freeze | grep -q "requests=="; then
          echo "The 'requests' library should not be a dependency of the project"
          false
        fi

glyph · 2026-06-02T07:11:55Z

Maybe we can have a CI task to check that 'requests' is not a dependency.

In this case a single manual test should work; we don't have ongoing regression tests to make sure we don't depend on NumPy or PyTorch for example :). The right way to do this is with constraints files, which we should eventually swtich to, but probably not as part of this change.

twm

Thank you for taking this on, @glyph! I'm a bit too tired to fully review tonight, but I noted an issue with treq.request() that should really be addressed before merge.

One other thought — doesn't the requests cookie jar also support assignment indexing? IIRC, to create a ☢️ supercookie ☢️. There are some other extensions, like get() and set() take domain and path args, etc.

twm · 2026-06-02T06:58:33Z

        :class:`hyperlink.EncodedURL` objects.
    """
-    return _client(kwargs).request(method, url, _stacklevel=3, **kwargs)
+    return _client(**kwargs).request(method, url, _stacklevel=3, **kwargs)


As _client() takes a different set of 4 arguments as .request() I don't think this'll work — it'll need the same re-declaration of all the arguments as get(), delete(), etc.

Huh. How come the tests didn't catch this?

It looks like there isn't any test that actually passes reactor. There's a similar issue if you pass agent, too.

twm · 2026-06-02T07:00:23Z

+
+R = TypeVar("R")
+
+SomeURL = Union[DecodedURL, EncodedURL, str, bytes]


Shouldn't this be _URLType? (Though SomeURL is a more pleasing name for it.)

Maybe _SomeURL? Should it go in _types? I don't have strong feelings.

twm · 2026-06-02T07:04:25Z

+    allows for indexing to retrieve cookie values.  This is for convenience and
+    for compatibility with `requests` users expectations.


I think that we should put a hazard warning sticker on this — it's dangerous to disregard the provenance of cookies in the face of HTTP redirects. IMO indexing by cookie name is dangerous enough that we should consider deprecating it, even as we do need it for compatibility. We provide the search() function as a safer interface.

twm · 2026-06-02T07:05:41Z

+
+        In general, you should not need to import or instantiate a
+        :py:class:`IndexableCookieJar` directly; anywhere that treq requires cookies, a
+        :py:class:`http.cookiejar.CookieJar` or ``dict`` of ``str`` to ``str``


Perhaps use the typing syntax to be succinct?

Suggested change

:py:class:`http.cookiejar.CookieJar` or ``dict`` of ``str`` to ``str``

:py:class:`http.cookiejar.CookieJar` or ``dict[str, str]``

twm · 2026-06-02T07:11:17Z

+.. autoclass:: IndexableCookieJar
+


Since users aren't expected to instantiate this, should it live below scoped_cookie() and search() in the docs?

No strong feelings from me on this one, but rearrange if you think it looks better.

twm · 2026-06-02T07:19:00Z

Oh no.

glyph · 2026-06-02T21:01:29Z

@twm Oops :). Do you think you could do a follow-up to address this stuff? I would rather intentionally leave off the supercookie-setitem; our own tests and examples don't depend on it. Should we deprecate the indexing support for similar reasons?

twm · 2026-06-04T06:37:13Z

@glyph I'm happy to follow up! I'd love to move this toward release.

I think that while removing the remainder of the it is technically an incompatible change, most of the behavior I mentioned is so hazardous an interface that I'd rather not support it at all. I think that we can settle for acknowledging the compatibility implications in the changelog.

glyph · 2026-06-04T07:37:30Z

@glyph I'm happy to follow up! I'd love to move this toward release.

I think that while removing the remainder of the it is technically an incompatible change, most of the behavior I mentioned is so hazardous an interface that I'd rather not support it at all. I think that we can settle for acknowledging the compatibility implications in the changelog.

Sounds good. I'll try to review when I can!

illustrate how to propagate type information through various asliases…

634edbd

… to get a correct type error

twm reviewed Sep 21, 2024

View reviewed changes

glyph added 5 commits May 30, 2026 15:20

Merge branch 'trunk' into no-more-requests

07fc00d

in the course of fixing a bunch of conflicts, also add a bunch of types

unused imports

3b20b9d

Merge remote-tracking branch 'origin/trunk' into no-more-requests

2cac3e9

more type fixes

0054d0f

lint fixes, twisted trunk no longer works with 3.9

5548b69

glyph marked this pull request as ready for review May 30, 2026 23:05

glyph requested a review from a team May 30, 2026 23:06

adiroiban reviewed May 31, 2026

View reviewed changes

Comment thread src/treq/cookies.py Outdated

Comment thread src/treq/client.py Outdated

glyph added 5 commits May 31, 2026 15:35

Kenneth has not been the maintainer of requests for almost 7 years

3ac2cfb

explain why this class exists.

ca02983

s/TreqieJar/IndexableCookieJar/g and add documentation for it

214a9be

no more requests please

edd757f

Proofreading the docs

1742f86

glyph added 3 commits May 31, 2026 16:28

save test-case-name

f8ab3f4

this is the terminal stack level so let's not pollute an extra param

6f513b7

twine should not be in normal env list

d1a346d

adiroiban approved these changes Jun 1, 2026

View reviewed changes

glyph merged commit 9fa973c into trunk Jun 2, 2026
17 checks passed

glyph deleted the no-more-requests branch June 2, 2026 07:12

twm reviewed Jun 2, 2026

View reviewed changes

twm mentioned this pull request Jun 6, 2026

Fix treq.request() argument handling #431

Draft

4 tasks

		return _client(kwargs).head(url, _stacklevel=4, **kwargs)


		@_like(HTTPClient.get)


		R = TypeVar("R")

		SomeURL = Union[DecodedURL, EncodedURL, str, bytes]

		allows for indexing to retrieve cookie values. This is for convenience and
		for compatibility with `requests` users expectations.

	:py:class:`http.cookiejar.CookieJar` or ``dict`` of ``str`` to ``str``
	:py:class:`http.cookiejar.CookieJar` or ``dict[str, str]``

Conversation

glyph commented Sep 19, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glyph commented May 30, 2026

Uh oh!

adiroiban left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

glyph commented May 31, 2026

Uh oh!

glyph commented May 31, 2026

Uh oh!

adiroiban left a comment

Choose a reason for hiding this comment

Uh oh!

glyph commented Jun 2, 2026

Uh oh!

Uh oh!

twm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twm commented Jun 2, 2026

Uh oh!

glyph commented Jun 2, 2026

Uh oh!

twm commented Jun 4, 2026

Uh oh!

glyph commented Jun 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants