praw-dev / prawcore Goto Github PK

Describe the Bug

Calls to subreddit.mod.stream.modqueue are failing with error message similar to this:

File "/home/ubuntu/src/src/prawcore/prawcore/rate_limit.py", line 79, in update
    self.remaining = float(response_headers["x-ratelimit-remaining"])
ValueError: could not convert string to float: '991, 98'

Desired Result

Ratelimit handling works without errors.

Code to reproduce the bug

#!/usr/bin/python3

import praw

SUB = "nasa"

def main():
    """Main loop"""

    reddit = praw.Reddit("nasamodqbot")
    subreddit = reddit.subreddit(SUB)

    for submission in subreddit.mod.stream.modqueue():
        print("Made it here")

if __name__ == "__main__":
    main()

My code does not include sensitive credentials

• Yes, I have removed sensitive credentials from my code.

Relevant Logs

Exception has occurred: ValueError
could not convert string to float: '962, 99'
  File "/home/david/src/r-nasabot/src/test99.py", line 16, in main
    for submission in subreddit.mod.stream.modqueue():
  File "/home/david/src/r-nasabot/src/test99.py", line 20, in <module>
    main()
ValueError: could not convert string to float: '962, 99'

This code has previously worked as intended

Yes

Operating System/Environment

Ubuntu 20.04, 22.04

Python Version

3.8.10, 3.10.12

prawcore Version

2.3.0

Links, references, and/or additional comments?

No response

See #44 (comment) for more information.

On 2.2.0 it works properly.

On replace_more is called occourse these error

   content.comments.replace_more(limit=None, threshold=3)
  File "../venv/lib/python3.9/site-packages/praw/models/reddit/base.py", line 34, in __getattr__
    self._fetch()
  File "../venv/lib/python3.9/site-packages/praw/models/reddit/submission.py", line 617, in _fetch
    data = self._fetch_data()
  File "../venv/lib/python3.9/site-packages/praw/models/reddit/submission.py", line 614, in _fetch_data
    return self._reddit.request("GET", path, params)
  File "../venv/lib/python3.9/site-packages/praw/reddit.py", line 856, in request
    return self._core.request(
  File "../venv/lib/python3.9/site-packages/prawcore/sessions.py", line 330, in request
    return self._request_with_retries(
  File "../venv/lib/python3.9/site-packages/prawcore/sessions.py", line 228, in _request_with_retries
    response, saved_exception = self._make_request(
  File "../venv/lib/python3.9/site-packages/prawcore/sessions.py", line 185, in _make_request
    response = self._rate_limiter.call(
  File "../venv/lib/python3.9/site-packages/prawcore/rate_limit.py", line 33, in call
    kwargs["headers"] = set_header_callback()
  File "../venv/lib/python3.9/site-packages/prawcore/sessions.py", line 283, in _set_header_callback
    self._authorizer.refresh()
  File "../venv/lib/python3.9/site-packages/prawcore/auth.py", line 371, in refresh
    if self._scopes:
AttributeError: 'ReadOnlyAuthorizer' object has no attribute '_scopes'

See: https://old.reddit.com/r/redditdev/comments/nw59ux/how_to_config_the_timeout/h1bd2w1/

Describe the Bug

Due to the use of Flit's fallback automatic metadata version extraction needing to dynamically import the package __init__.py instead of reading it statically (see pypa/flit#386 ) , since the actual __version__ is imported from const.py, this requires requests be installed in the build environment when building or installing from an sdist (or the source tree).

This happens merely by chance to currently be a transitive backend dependency of flit_core and thus building in isolated mode works. However, this shouldn't be relied upon, as if either flit_core or prawcore's dependencies were to ever change, this would break isolated builds too. And requests isn't an exposed non-backend dependency of flit-core, so it doesn't actually get installed otherwise.

This means Requests must be manually installed (not just the explicitly pyproject,toml-specified build dependencies) if building/installing prawcore in any other context than pip's isolated mode, e.g. without the --no-build-isolation flag, via the legacy non-PEP 517 builder, or via other tools or most downstream ecosystems that use other build isolation mechanisms. In particular, this was an issue on conda-forge/prawcore-feedstock#14 where I was packaging the new prawcore 2.4.0 version for Conda-Forge—you can see the full Azure build log.

Of course, this can be worked around for now by manually installing requests into the build environment, but that's certainly not an ideal solution as it adds an extra build dependency (and its dependencies in turn), requires extra work by everyone installing from source (without build isolation), makes builds take longer, and is fragile and not easily maintainable/scalable long term for other runtime dependencies.

Desired Result

Prawcore is able to be built and installed without installing requests, or relying on it happening to be a transitive build backend dependency of flit_core and present "by accident".

There are several ways to achieve this, all potentially reasonable options:

• Move __version__ to be in __init__.py directly, so Flit can find it by static inspection without triggering an import
• Include version as static metadata in the pyproject.toml instead of relying on dynamic backend-specific detection
• Refactor __init__.py to be as minimal as possible and avoid inefficiently importing your entire package (or only do so lazily), as generally recommended.

Code to reproduce the bug

# In a fresh venv without requests or prawcore installed
pip install flit-core  # Or flit
pip install --no-build-isolation prawcore --no-binary prawcore

The Reddit() initialization in my code example does not include the following parameters to prevent credential leakage:

client_secret, password, or refresh_token.

• Yes

Relevant Logs

### Logs from local pip install of sdist ###

$ pip install --no-build-isolation prawcore --no-binary prawcore
DEPRECATION: --no-binary currently disables reading from the cache of locally built wheels. In the future --no-binary will not influence the wheel cache. pip 23.1 will enforce this behaviour change. A possible replacement is to use the --no-cache-dir option. You can use the flag --use-feature=no-binary-enable-wheel-cache to test the upcoming behaviour. Discussion can be found at https://github.com/pypa/pip/issues/11453
Collecting prawcore
  Using cached prawcore-2.4.0.tar.gz (15 kB)
  Preparing metadata (pyproject.toml) ... error
  error: subprocess-exited-with-error

  × Preparing metadata (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [22 lines of output]
      Traceback (most recent call last):
        File "C:\Miniconda3\envs\qt6-env\lib\site-packages\pip\_vendor\pyproject_hooks\_in_process\_in_process.py", line 353, in <module>
          main()
        File "C:\Miniconda3\envs\qt6-env\lib\site-packages\pip\_vendor\pyproject_hooks\_in_process\_in_process.py", line 335, in main
          json_out['return_val'] = hook(**hook_input['kwargs'])
        File "C:\Miniconda3\envs\qt6-env\lib\site-packages\pip\_vendor\pyproject_hooks\_in_process\_in_process.py", line 149, in prepare_metadata_for_build_wheel
          return hook(metadata_directory, config_settings)
        File "C:\Miniconda3\envs\qt6-env\lib\site-packages\flit_core\buildapi.py", line 49, in prepare_metadata_for_build_wheel
          metadata = make_metadata(module, ini_info)
        File "C:\Miniconda3\envs\qt6-env\lib\site-packages\flit_core\common.py", line 425, in make_metadata
          md_dict.update(get_info_from_module(module, ini_info.dynamic_metadata))
        File "C:\Miniconda3\envs\qt6-env\lib\site-packages\flit_core\common.py", line 222, in get_info_from_module
          docstring, version = get_docstring_and_version_via_import(target)
        File "C:\Miniconda3\envs\qt6-env\lib\site-packages\flit_core\common.py", line 195, in get_docstring_and_version_via_import
          spec.loader.exec_module(m)
        File "<frozen importlib._bootstrap_external>", line 883, in exec_module
        File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
        File "C:\Users\C. A. M. Gerlach\AppData\Local\Temp\pip-install-1uivtksa\prawcore_cee755c9a2dc47b29dfbe46b846de281\prawcore\__init__.py", line 5, in <module>
          from .auth import (
        File "C:\Users\C. A. M. Gerlach\AppData\Local\Temp\pip-install-1uivtksa\prawcore_cee755c9a2dc47b29dfbe46b846de281\prawcore\auth.py", line 8, in <module>
          from requests import Request
      ModuleNotFoundError: No module named 'requests'
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

× Encountered error while generating package metadata.
╰─> See above for output.

note: This is an issue with the package mentioned above, not pip.
hint: See above for details.

This code has previously worked as intended.

Yes

Operating System/Environment

Any, tested Linux, Windows

Python Version

Any; tested 3.10, 3.12

prawcore Version

2.4.0 (newely introduced due to switch to Flit)

Anything else?

Just for reference, this was the build environment on Azure:

    _libgcc_mutex:    0.1-conda_forge           conda-forge
    _openmp_mutex:    4.5-2_gnu                 conda-forge
    bzip2:            1.0.8-h7f98852_4          conda-forge
    ca-certificates:  2023.7.22-hbcca054_0      conda-forge
    flit-core:        3.9.0-pyhd8ed1ab_0        conda-forge
    ld_impl_linux-64: 2.40-h41732ed_0           conda-forge
    libexpat:         2.5.0-hcb278e6_1          conda-forge
    libffi:           3.4.2-h7f98852_5          conda-forge
    libgcc-ng:        13.2.0-h807b86a_2         conda-forge
    libgomp:          13.2.0-h807b86a_2         conda-forge
    libnsl:           2.0.1-hd590300_0          conda-forge
    libsqlite:        3.43.2-h2797004_0         conda-forge
    libuuid:          2.38.1-h0b41bf4_0         conda-forge
    libzlib:          1.2.13-hd590300_5         conda-forge
    ncurses:          6.4-hcb278e6_0            conda-forge
    openssl:          3.1.3-hd590300_0          conda-forge
    pip:              23.3-pyhd8ed1ab_0         conda-forge
    python:           3.12.0-hab00c5b_0_cpython conda-forge
    readline:         8.2-h8228510_1            conda-forge
    setuptools:       68.2.2-pyhd8ed1ab_0       conda-forge
    tk:               8.6.13-h2797004_0         conda-forge
    tzdata:           2023c-h71feb2d_0          conda-forge
    wheel:            0.41.2-pyhd8ed1ab_0       conda-forge
    xz:               5.2.6-h166bdaf_0          conda-forge

And this was the relevant portion of the build log:

### Logs from Azure build of Conda-Forge package ###

# https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=806405&view=logs&jobId=656edd35-690f-5c53-9ba3-09c10d0bea97&j=656edd35-690f-5c53-9ba3-09c10d0bea97&t=986b1512-c876-5f92-0d81-ba851554a0a3

Source cache directory is: /home/conda/feedstock_root/build_artifacts/src_cache
Downloading source to cache: prawcore-2.4.0_b7b2b5a1d0.tar.gz
Downloading https://pypi.io/packages/source/p/prawcore/prawcore-2.4.0.tar.gz
INFO:conda_build.source:Success
Success
Extracting download
source tree in: /home/conda/feedstock_root/build_artifacts/prawcore_1697605563769/work
export PREFIX=/home/conda/feedstock_root/build_artifacts/prawcore_1697605563769/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pl
export BUILD_PREFIX=/home/conda/feedstock_root/build_artifacts/prawcore_1697605563769/_build_env
export SRC_DIR=/home/conda/feedstock_root/build_artifacts/prawcore_1697605563769/work
Using pip 23.3 from $PREFIX/lib/python3.12/site-packages/pip (python 3.12)
Non-user install because user site-packages disabled
Ignoring indexes: https://pypi.org/simple
Created temporary directory: /tmp/pip-build-tracker-h18miru8
Initialized build tracking at /tmp/pip-build-tracker-h18miru8
Created build tracker: /tmp/pip-build-tracker-h18miru8
Entered build tracker: /tmp/pip-build-tracker-h18miru8
Created temporary directory: /tmp/pip-install-qe_g7qir
Created temporary directory: /tmp/pip-ephem-wheel-cache-gmhw8jmc
Processing $SRC_DIR
  Added file://$SRC_DIR to build tracker '/tmp/pip-build-tracker-h18miru8'
  Created temporary directory: /tmp/pip-modern-metadata-h0yhew9i
  Preparing metadata (pyproject.toml): started
  Running command Preparing metadata (pyproject.toml)
  Traceback (most recent call last):
    File "/home/conda/feedstock_root/build_artifacts/prawcore_1697605563769/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pl/lib/python3.12/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 353, in <module>
      main()
    File "/home/conda/feedstock_root/build_artifacts/prawcore_1697605563769/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pl/lib/python3.12/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 335, in main
                              ^^^^^^^^^^^^^^^^^^
  File "$PREFIX/lib/python3.12/site-packages/pip/_internal/operations/build/metadata.py", line 37, in generate_metadata
    raise MetadataGenerationFailed(package_details=details) from error
pip._internal.exceptions.MetadataGenerationFailed: metadata generation failed

[Deleted]

Commit 342e3e8 introducees Authenticator._validate_authenticator(). If I understand correctly, that validator prevents users to use code grant flow from installed clients. Since many distributed desktop applications uses code grant flow (with optional refresh token), I believe that validator could be a problem.

The vast majority of exceptions that prawcore raises to my bot are 503 and 504 status codes when Reddit is experiencing outages. In this case, the only sensible thing to do is to wait a while and try again.

I think it would make sense for prawcore to provide this behavior as an option, rather than re-implementing it in every client that wants this behavior.

It probably should be optional, so that (e.g.) applications with user interfaces can just report the outage instead of stalling indefinitely.

Does it sound right for prawcore to do this?

I am having the following error when running the test suites. Same errors are present in Python 2.7 and 3.5, too.

Other version info:
betamax-matchers 0.3.0
betamax-serializers 0.2.0
requests 2.12.4

======================================================================
ERROR: test_request__patch (tests.test_sessions.SessionTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/build/python-prawcore/src/prawcore/prawcore/requestor.py", line 46, in request
    return self._http.request(*args, timeout=TIMEOUT, **kwargs)
  File "/usr/lib/python3.6/site-packages/requests/sessions.py", line 488, in request
    resp = self.send(prep, **send_kwargs)
  File "/usr/lib/python3.6/site-packages/requests/sessions.py", line 609, in send
    r = adapter.send(request, **kwargs)
  File "/usr/lib/python3.6/site-packages/betamax/adapter.py", line 132, in send
    current_cassette))
betamax.exceptions.BetamaxError: A request was made that could not be handled.

A request was made to https://www.reddit.com/api/v1/access_token that could not be found in Session_request__patch.

The settings on the cassette are:

    - record_mode: once
    - match_options {'json-body', 'method', 'uri'}.


During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/build/python-prawcore/src/prawcore/tests/test_sessions.py", line 77, in test_request__patch
    session = prawcore.Session(script_authorizer())
  File "/build/python-prawcore/src/prawcore/tests/test_sessions.py", line 41, in script_authorizer
    authorizer.refresh()
  File "/build/python-prawcore/src/prawcore/prawcore/auth.py", line 328, in refresh
    password=self._password)
  File "/build/python-prawcore/src/prawcore/prawcore/auth.py", line 138, in _request_token
    response = self._authenticator._post(url, **data)
  File "/build/python-prawcore/src/prawcore/prawcore/auth.py", line 29, in _post
    data=sorted(data.items()))
  File "/build/python-prawcore/src/prawcore/prawcore/requestor.py", line 48, in request
    raise RequestException(exc, args, kwargs)
prawcore.exceptions.RequestException: error with request A request was made that could not be handled.

A request was made to https://www.reddit.com/api/v1/access_token that could not be found in Session_request__patch.

The settings on the cassette are:

    - record_mode: once
    - match_options {'json-body', 'method', 'uri'}.


----------------------------------------------------------------------
Ran 80 tests in 26.563s

FAILED (errors=1)

Currently the sleep time between requests is calculated based purely on the requests remaining and the seconds to reset, and doesn't take into account the time it takes for the request itself.

As an example, my tests are averaging 0.65 seconds for the request itself to return. So right after the reset, with 600 requests and 600 seconds, praw will wait 1 second, then the request takes 0.65 seconds. If I hit the middle of the window, say 600 requests left and 300 seconds, it will wait 0.5 seconds, then the request takes 0.65 seconds, which still results in a total wait time of 1.15 seconds.

So in 600 seconds you end up sending something like 350-400 requests.

I think this should be changed to have no sleep time at all if the number of requests remaining is greater than the number of seconds until reset. Or possibly a bit of buffer, if the requests remaining is, say, more than 10 above the number of seconds until reset. Just to make sure there's no edge case where you end up waiting 10 seconds for something.

The wiki endpoint can return 415 response status code for a few reasons:

• Account has _spam field set to True
• Editing config/stylesheet and there are css syntax issues
• Editing config/automoderator and there are syntax issues

A SpecialError class should be returned providing the message(s) that are included in the 415 response.

Please add public settings for changing the number of retries (e.g., an environment variable and Python variable similar to prawcore_timeout and prawcore.requestor.TIMEOUT).

reddit._core._retry_strategy_class._retries = N is the only way to change it right now.

Thanks!

Describe the solution you'd like

I updated the rate limiting code a couple years ago in #89, but reddit's recent changes have exposed some limitations in the algorithm.

When the requests per window is very low, like reddit's new unauthenticated limit of 10 requests per minute, the algorithm waits its default maximum of 10 seconds between requests until it "catches up". And when the requests per window is high, like the new rate limit of 100 per minute, it runs through requests as quickly as possible and then abruptly slows down once it again catches up. In both cases it should ideally use requests at a steady rate the through the whole window.

I made several attempts at a new algorithm and was going to have a spreadsheet demonstrating the improvements like I did last time I brought this up, but I ended up not being satisfied with anything I came up with. Fixing the problems above is compounded by the issues I fixed last time, starting requests in the middle of a reset window and there being multiple clients making requests at the same time.

So I wanted to bring up an idea before I put more time into it. The current approach only uses the values from the previous request. @bboe what would you think of keeping more state, a running average using a couple recent requests to help estimate how many requests per second are being used and how long to wait.

If you don't think that's a terrible idea I can mock something up for you to take a look at.

Describe alternatives you've considered

No response

Additional context

No response

Describe the solution you'd like

prawcore is currently tied to requests and asyncprawcore to aiohttp. I like how httpx has both async and sync capabilities and would like to use one HTTP library in my project as I will be calling other APIs as well. I would also like to see if httpx's HTTP/2 implementation provides any latency improvement over HTTP/1.1 implementations.

httpx is broadly compatible with requests but httpx.Client does not implement the exact API as requests.Session: https://www.python-httpx.org/compatibility/

Naively constructing praw.Reddit with requestor_kwargs={"session": client} where client = Client(http2=True), I was able to run praw's quickstart example listing 10 hot submissions with the below two patches:

replace with data=data, httpx only supports dict syntax and not a list of tuples

remove entirely, as httpx names the parameter differently and defaults to no redirects anyway

Describe alternatives you've considered

No response

Additional context

https://praw.slack.com/archives/CRZ00QLC9/p1681666057106319

Please do as title