Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #326 (hopefully)
Changes proposed in this pull request:
beaker-py
to minimum v1.6.2 in order to get this essential bug fix: allenai/beaker-py@d3f18dc.BeakerWorkspace
.requests.Session()
object within the Beaker client to address the "too many open files" issue ("Too many open files" while usingBeakerExecutor
#326) and improve performance.requests.Session()
is thread safe for this use-case, even though the maintainers ofrequests
don't provide any guarantees of thread safety. That said, following the discussion in psf/requests#2766, it seems the main reason for concern is the lack of thread safety inurllib3.PoolManager
(psf/requests#2766 (comment)). The thread-safety issue with the pool manager seems to boil down to this: urllib3/urllib3#1252 (comment). That is, when the number of hosts is greater than the number of pools. But the default number of pools is 10 (https://github.com/psf/requests/blob/main/requests/adapters.py#L66), and in our case there are only two different hosts that we connect to: beaker.org and the Beaker file heap server. So we should be okay 🤷♂️If we do run into issues in the future, we should handle this in
beaker-py
but using one session per thread. We could implement this using thread-local storage similar to this: https://github.com/customerio/customerio-python/pull/77/filesBefore submitting
section of the
CONTRIBUTING
docs.Writing docstrings section of the
CONTRIBUTING
docs.After submitting