build(datasets): Bump s3fs

[merelcht]

Description

Different approach to #348 to bump s3fs, to solve failing tests in:

• build(datasets): upgrade s3fs to newer calver #348
• build: Experiment aws patching #458

The problem that surfaced in #348 is described here: aio-libs/aiobotocore#755 Lots of people have provided patches, so I have used one of the most up to date ones and it seems to work almost perfectly.

Development notes

Tests that are still broken and have been commented out for the time being:

• tests/dask/test_parquet_dataset.py test_pass_credentials
• tests/spark/test_spark_dataset.py test_no_version

See #459 for some more things I tried to get these tests passing. We just need to change the way the mocking is done in these, but I'm out of ideas at the moment. Probably won't be too hard to fix with a fresh pair of eyes.

Checklist

• Opened this PR as a 'Draft Pull Request' if it is work-in-progress
• Updated the documentation to reflect the code changes
• Added a description of this change in the relevant RELEASE.md file
• Added tests to cover my changes

[MatthiasRoels]

Thanks @merelcht for picking it up and fixing the failing tests! This approach is cleaner than what I initially thought it would.

I still believe my approach (with moto in server mode) could work, but I wasn't able to figure out why it didn't. The s3fs package is using it for their tests, so if I have time, I will look into their approach to see what they do differently than what I had.

[merelcht]

Thanks @merelcht for picking it up and fixing the failing tests! This approach is cleaner than what I initially thought it would.

I still believe my approach (with moto in server mode) could work, but I wasn't able to figure out why it didn't. The s3fs package is using it for their tests, so if I have time, I will look into their approach to see what they do differently than what I had.

Thanks @MatthiasRoels that sounds good! We want to get the new version bump of s3fs into the next breaking datasets release, but when it's in we can always change our testing approach 🙂

[SajidAlamQB]

LGTM overall just left a minor comment, thank you @merelcht! 👍

[SajidAlamQB]

Should read be in __init__? I would have thought it would be in MockHttpClientResponse and not in the __init__.

[merelcht]

Good question, tbh I didn't try other variations of this specific patch. I used the most recent and clean snippet I could find in the threads here aio-libs/aiobotocore#755 https://gist.github.com/giles-betteromics/12e68b88e261402fbe31c2e918ea4168, assuming that writing my own wasn't going to be better than all the iterations from the community.

[DimedS]

Thank you, @merelcht , good solution!