Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't close user-provided requests sessions #232

Merged
merged 2 commits into from Jun 19, 2023

Conversation

reconman
Copy link
Contributor

Description

If the user has provided a requests session in the RequestsEndpoint constructor, it will not be closed after each request anymore.

If no session was provided, one will be created in the constructor and closed in the destructor.

Related Issues

Closes #225

Pull request checklist

  • Tests: This PR includes tests for covering the features or bug fixes (if applicable).
  • Docs: This PR updates/creates the necessary documentation.
  • CI: Make sure your Pull Request passes all CI checks. If not, clarify the motif behind that and the action plan to solve it (may reference a ticket)

How to test it

Change the Github Agile Dashboard example the following way

  1. Replace the HTTPEndpoint with a RequestsEndpoint
  2. Enable debug logging, especially for the urllib3.connectionpool logger
  3. Check the log for "Starting new HTTPS connection" - it should only appear once

@@ -109,7 +109,13 @@ def __init__(self, url, base_headers=None, timeout=None, method='POST',
self.timeout = timeout
self.method = method
self.auth = auth
self.session = session
self.session = session or requests.Session()
self.close_session = session is not None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this flipped? You want to close if or requests.Session(), in this case session is None

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad

@barbieri
Copy link
Member

failing tests checks the string serialization without session. Since these will be the case now (every call will have a session), just change the test at https://github.com/profusion/sgqlc/blob/master/tests/test-endpoint-requests.py

@reconman
Copy link
Contributor Author

failing tests checks the string serialization without session. Since these will be the case now (every call will have a session), just change the test at https://github.com/profusion/sgqlc/blob/master/tests/test-endpoint-requests.py

Tried to execute the test on windows, but it horrible fails in the initialization:

Traceback (most recent call last):
  File "C:\Github\sgqlc\setup.py", line 23, in <module>
  File "C:\Github\sgqlc\venv\Lib\site-packages\setuptools\__init__.py", line 87, in setup
  File "C:\Github\sgqlc\venv\Lib\site-packages\setuptools\_distutils\core.py", line 185, in setup
  File "C:\Github\sgqlc\venv\Lib\site-packages\setuptools\_distutils\core.py", line 201, in run_commands
  File "C:\Github\sgqlc\venv\Lib\site-packages\setuptools\_distutils\dist.py", line 968, in run_commands
  File "C:\Github\sgqlc\venv\Lib\site-packages\setuptools\dist.py", line 1217, in run_command
  File "C:\Github\sgqlc\venv\Lib\site-packages\setuptools\_distutils\dist.py", line 987, in run_command
  File "C:\Github\sgqlc\venv\Lib\site-packages\nose\commands.py", line 158, in run
  File "C:\Github\sgqlc\venv\Lib\site-packages\nose\core.py", line 118, in __init__
  File "C:\Program Files\Python311\Lib\unittest\main.py", line 101, in __init__
  File "C:\Github\sgqlc\venv\Lib\site-packages\nose\core.py", line 145, in parseArgs
  File "C:\Github\sgqlc\venv\Lib\site-packages\nose\config.py", line 346, in configure
  File "C:\Github\sgqlc\venv\Lib\site-packages\nose\plugins\manager.py", line 284, in configure
  File "C:\Github\sgqlc\venv\Lib\site-packages\nose\plugins\manager.py", line 99, in __call__
  File "C:\Github\sgqlc\venv\Lib\site-packages\nose\plugins\manager.py", line 167, in simple
  File "C:\Github\sgqlc\venv\Lib\site-packages\nose\plugins\cover.py", line 156, in configure
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\control.py", line 565, in start
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\collector.py", line 329, in start
  File "C:\Program Files\Python311\Lib\threading.py", line 72, in settrace
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\control.py", line 334, in _should_trace
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\inorout.py", line 321, in should_trace
TypeError: startswith first arg must be bytes or a tuple of bytes, not str
Exception ignored in: <object repr() failed>
Traceback (most recent call last):
  File "C:\Program Files\Python311\Lib\threading.py", line 1534, in _shutdown
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\control.py", line 334, in _should_trace
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\inorout.py", line 321, in should_trace
TypeError: startswith first arg must be bytes or a tuple of bytes, not str
Exception ignored in atexit callback: <bound method Coverage._atexit of <coverage.control.Coverage object at 0x0000020C17043250>>
Traceback (most recent call last):
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\control.py", line 578, in _atexit
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\control.py", line 334, in _should_trace
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\inorout.py", line 321, in should_trace
TypeError: startswith first arg must be bytes or a tuple of bytes, not str
Exception ignored in atexit callback: <function shutdown at 0x0000020C1440BD80>
Traceback (most recent call last):
  File "C:\Program Files\Python311\Lib\logging\__init__.py", line 2177, in shutdown
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\control.py", line 334, in _should_trace
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\inorout.py", line 321, in should_trace
TypeError: startswith first arg must be bytes or a tuple of bytes, not str
Exception ignored in: <function Forward.__del__ at 0x0000020C16024360>
Traceback (most recent call last):
  File "C:\Github\sgqlc\venv\Lib\site-packages\setuptools\_vendor\pyparsing\core.py", line 5186, in __del__
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\control.py", line 334, in _should_trace
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\inorout.py", line 321, in should_trace
TypeError: startswith first arg must be bytes or a tuple of bytes, not str
Exception ignored in: <function Forward.__del__ at 0x0000020C15C9A980>
Traceback (most recent call last):
  File "C:\Github\sgqlc\venv\Lib\site-packages\pkg_resources\_vendor\pyparsing\core.py", line 5186, in __del__
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\control.py", line 334, in _should_trace
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\inorout.py", line 321, in should_trace
TypeError: startswith first arg must be bytes or a tuple of bytes, not str
Exception ignored in: <function _removeHandlerRef at 0x0000020C14403B00>
Traceback (most recent call last):
  File "C:\Program Files\Python311\Lib\logging\__init__.py", line 845, in _removeHandlerRef
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\control.py", line 334, in _should_trace
  File "C:\Github\sgqlc\venv\Lib\site-packages\coverage\inorout.py", line 321, in should_trace
TypeError: startswith first arg must be bytes or a tuple of bytes, not str

So I'll have to rely on Github actions to check if the tests are fixed.

@reconman reconman requested a review from barbieri June 19, 2023 19:02
@coveralls
Copy link

coveralls commented Jun 19, 2023

Pull Request Test Coverage Report for Build 5315290371

  • 15 of 15 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 5192762666: 0.0%
Covered Lines: 1599
Relevant Lines: 1599

💛 - Coveralls

@barbieri barbieri merged commit 0475f38 into profusion:master Jun 19, 2023
3 checks passed
@barbieri
Copy link
Member

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connections in requests session are closed after each request
3 participants