test: s3fs pytest unraisable exception #1012
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ec-optional-backends * origin/source-futures-submit: Future init do not use named arguments for the path/url annotation update submit interface
lobis
changed the title
jpivarski
approved these changes
Nov 16, 2023
Yes, before I gave up on fixing the underlying issue I tried many different things. I'm curious, is there a problem I'm missing with the long list of commits? I'll get squashed, it'll just be a very long commit message, am I right? |
|
That's right. It will be a long description; the title of the commit will be the title of the PR plus " (#1012)". It's only a problem for mouseover in the GitHub web interface, which shows the long description of the commit you're hovering over. |
lobis
added a commit
that referenced
this pull request
Nov 16, 2023
* install optional fsspec backends in the CI for some python versions * update submit interface * annotation * do not use named arguments for the path/url * Future init * add s3fs and sshfs as test dependencies (fsspec beckends) * test fsspec s3 for more combination of parameters * remove pip install from ci * style: pre-commit fixes * revert test order * remove dependencies as a test * add s3fs to test * exclude s3fs to python version 3.12 * add sshfs test (skipped) * fix pytest * asyncio not available in 3.12 * asyncio not available in 3.12 * add comment for fsspec threads * attempt to close resources * handle s3fs case separate for now * attempt to pass tests * attempt to pass tests * simplified * remove support for use_threads option, run non-async fs in threads using asyncio * stop the loop on resource shutdown * add skip for xrootd due to server issues * remove skip for xrootd * remove shutdown * understand ci fail * enable test that uses s3fs * skip test for CI * skip tests on problematic python versions * test S3Source only in its test file * fix pytest parameter * rename test * rename test function (duplicate name) * imports * rerun on timeout * rerun on timeout * remove minio * skip s3fs test instead * rename test * no s3 in test names * improve pytest regex * python 3.8 skip * skip s3_fail when timeout * error tuple * line jump --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Jim Pivarski <jpivarski@users.noreply.github.com>
nsmith-
reviewed
May 2, 2024
Comment on lines
+64
to
+67
| if sys.version_info < (3, 11): | ||
| pytest.skip( | ||
| "https://github.com/scikit-hep/uproot5/pull/1012", | ||
| ) |
There was a problem hiding this comment.
@lobis why specifically versions less than 3.11?
There was a problem hiding this comment.
No particular reason, it just failed on older versions, I never could find out why.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR doesn't fix the underlying cause of this error in pytest but it makes it go away from the CI.
S3Sourcetests. There is an issue with the test that checks for exception when the file does not exist: many times this throws a timeout that doesn't go away with retries, I will just add the timeout exceptions to the list of valid exceptions.s3fstest but only on python 3.11 and above. In older python version we have this mysterious error.I'm 90% confident that this mysterious error won't cause a problem when using uproot with s3 and fsspec but it's the reason I didn't set it as default in #1023. Now that I think about it, I don't think it was a good idea to not set the s3 default to s3fs. It probably works fine and doing so will probably let us know if this is the case, so I will do it (in another PR).