Bug 250941 - www/py-requests: Update to 2.25.1
Summary: www/py-requests: Update to 2.25.1
Status: In Progress
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Kubilay Kocak
URL: https://github.com/psf/requests/blob/...
Keywords: needs-patch, needs-qa
Depends on: 252187
Blocks: 250923 252180
  Show dependency treegraph
 
Reported: 2020-11-08 07:53 UTC by Olivier Cochard
Modified: 2021-02-10 09:38 UTC (History)
2 users (show)

See Also:
koobs: maintainer-feedback+


Attachments
Patch to update to 2.24.0 (3.84 KB, patch)
2020-11-08 07:53 UTC, Olivier Cochard
no flags Details | Diff
2.25.1 v1 (4.84 KB, patch)
2020-12-27 06:41 UTC, Charlie Li
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Olivier Cochard freebsd_committer 2020-11-08 07:53:44 UTC
Created attachment 219446 [details]
Patch to update to 2.24.0

net/py-GeoIP2 requires www/py-requests 2.24.0.
So here is a patch to update it.
Comment 1 Olivier Cochard freebsd_committer 2020-11-16 09:13:43 UTC
Any news about this PR ?
Comment 2 Olivier Cochard freebsd_committer 2020-12-02 21:51:02 UTC
Any news?
Comment 3 Kubilay Kocak freebsd_committer freebsd_triage 2020-12-08 05:50:51 UTC
This needs comprehensive QA, in particular for dependency compatibility and reverse dependents.
Comment 4 Charlie Li 2020-12-26 23:40:26 UTC
We're up to 2.25.1 now.

The current version (upstream and in ports) of devel/py-cookiecutter needs at least 2.23.0 to run, as another consumer data point.
Comment 5 Kubilay Kocak freebsd_committer freebsd_triage 2020-12-27 01:22:16 UTC
(In reply to Charlie Li from comment #4)

Currently wrestling with the test suite (which is crashing). Needed to complete QA
Comment 6 Charlie Li 2020-12-27 01:51:29 UTC
(In reply to Kubilay Kocak from comment #5)
Same. Noticed that it was downloading dependencies on its own using Python tooling before any tests had a chance to crash.
Comment 7 Kubilay Kocak freebsd_committer freebsd_triage 2020-12-27 01:54:28 UTC
(In reply to Charlie Li from comment #6)

setup.py test downloads deps (via tests_require), which was very convenienent, particularly for un-ported test dependencies (eg: requests wants pytest-httpbin, very old pinned version)

setup.py test is 'deprecated' (but pypa hasn't provided a canonical replacement that does the same)

pytest et al don't install dependencies defined in tests_require, which is annoying.

Would love help diagnosing/isolating the test crash issue (related to pytest --boxed multiprocessing)
Comment 8 Charlie Li 2020-12-27 02:15:36 UTC
Their CI got moved to GitHub Actions, and it appears that they install dependencies and run pytest separately. Probably best to run it from their Makefile.
Comment 9 Kubilay Kocak freebsd_committer freebsd_triage 2020-12-27 02:22:44 UTC
(In reply to Charlie Li from comment #8)

They run pytest tests, whereas I was running pytest without working dir. Will try.
Comment 10 Kubilay Kocak freebsd_committer freebsd_triage 2020-12-27 02:25:03 UTC
(In reply to Kubilay Kocak from comment #9)

This is slightly better, but need to resolve (157 instances of the error):

  E       recursive dependency involving fixture 'httpbin' detected

Found one google result on this with no indicated specific resolution
Comment 11 Kubilay Kocak freebsd_committer freebsd_triage 2020-12-27 03:23:45 UTC
Yakception:

pytest-httpbin (NEW PORT)
httpbin (NEW PORT)

 9 failed, 522 passed, 12 skipped, 1 xfailed, 14 warnings in 57.28 seconds

Several "requests.exceptions.InvalidSchema: Missing dependencies for SOCKS support" to resolve

Plus a few more.
Comment 12 Kubilay Kocak freebsd_committer freebsd_triage 2020-12-27 03:27:23 UTC
1 failed, 530 passed, 12 skipped, 1 xfailed, 14 warnings in 61.34 seconds

____________ TestRequests.test_https_warnings _____________________________
self = <tests.test_requests.TestRequests object at 0x804ad9490>, httpbin_secure = <function prepare_url.<locals>.inner at 0x805d6c4c0>, httpbin_ca_bundle = None

    def test_https_warnings(self, httpbin_secure, httpbin_ca_bundle):
        """warnings are emitted with requests.get"""
        if HAS_MODERN_SSL or HAS_PYOPENSSL:
            warnings_expected = ('SubjectAltNameWarning', )
        else:
            warnings_expected = ('SNIMissingWarning',
                                 'InsecurePlatformWarning',
                                 'SubjectAltNameWarning', )

        with pytest.warns(None) as warning_records:
            warnings.simplefilter('always')
            requests.get(httpbin_secure('status', '200'),
                         verify=httpbin_ca_bundle)

        warning_records = [item for item in warning_records
                           if item.category.__name__ != 'ResourceWarning']

        warnings_category = tuple(
            item.category.__name__ for item in warning_records)
>       assert warnings_category == warnings_expected
E       AssertionError: assert () == ('SubjectAltNameWarning',)
E         Right contains one more item: 'SubjectAltNameWarning'
E         Full diff:
E         - ()
E         + ('SubjectAltNameWarning',)

tests/test_requests.py:857: AssertionError
Comment 13 Kubilay Kocak freebsd_committer freebsd_triage 2020-12-27 03:32:17 UTC
(In reply to Kubilay Kocak from comment #12)

https://github.com/psf/requests/issues/5530

Will exclude the test.
Comment 14 Charlie Li 2020-12-27 04:27:46 UTC
There may or may not be a bug with test_requests with current versions of httpbin and pytest-httpbin. Removing the SubjectAltNameWarning from the warnings_expected tuple allows the test to pass:

--- tests/test_requests.py.orig 2020-12-16 17:43:25 UTC
+++ tests/test_requests.py
@@ -838,7 +838,7 @@ class TestRequests:
     def test_https_warnings(self, httpbin_secure, httpbin_ca_bundle):
         """warnings are emitted with requests.get"""
         if HAS_MODERN_SSL or HAS_PYOPENSSL:
-            warnings_expected = ('SubjectAltNameWarning', )
+            warnings_expected = ()
         else:
             warnings_expected = ('SNIMissingWarning',
                                  'InsecurePlatformWarning',

531 passed, 12 skipped, 1 xfailed in 63.20 seconds
Comment 15 Kubilay Kocak freebsd_committer freebsd_triage 2020-12-27 06:29:43 UTC
yakception [1]. I'm currently 4 levels deep, do not know when I'll come back from this, or how deep this rabbithole goes

[1] requests -> pytest-httpbin (new port) -> httpbin (new port) -> flasgger (new port) -> apispec-webframeworks (new port) | flask-jwt (new port) ...
Comment 16 Charlie Li 2020-12-27 06:36:03 UTC
Apart from httpbin (bug 251847) and pytest-httpbin (bug 252187), the other new ports are not needed to run the tests.
Comment 17 Charlie Li 2020-12-27 06:41:12 UTC
Created attachment 220983 [details]
2.25.1 v1

This one has all tests passing, with the slight adjustment included. Note that httpbin and pytest-httpbin are in TEST_DEPENDS.
Comment 18 Kubilay Kocak freebsd_committer freebsd_triage 2020-12-27 07:21:16 UTC
(In reply to Charlie Li from comment #16)

Thanks Charlie. I should have made it clearer that I already have all of comment 15 ported so far
Comment 19 Kubilay Kocak freebsd_committer freebsd_triage 2020-12-27 07:22:09 UTC
(In reply to Charlie Li from comment #16)

flasgger is required for httpbin to be completed correctly, it imports it at runtime
Comment 20 Kubilay Kocak freebsd_committer freebsd_triage 2020-12-28 03:04:13 UTC
DONE: requests -> pytest-httpbin -> httpbin -> flasgger -> pispec-webframeworks | flask-jwt

latest requests test: 531 passed, 12 skipped, 1 xfailed, 14 warnings in 61.24 seconds

Pending reverse dependents test