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

Bump s3fs version #2017

Closed
wants to merge 14 commits into from
Closed

Bump s3fs version #2017

wants to merge 14 commits into from

Conversation

noklam
Copy link
Contributor

@noklam noklam commented Nov 10, 2022

NOTE: Kedro datasets are moving from kedro.extras.datasets to a separate kedro-datasets package in
kedro-plugins repository. Any changes to the dataset implementations
should be done by opening a pull request in that repository.

Description

Potentially fix #1995

Development notes

Checklist

  • Read the contributing guidelines
  • 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 RELEASE.md file
  • Added tests to cover my changes

@noklam noklam marked this pull request as ready for review November 11, 2022 11:26
@noklam noklam requested a review from idanov as a code owner November 11, 2022 11:26
@noklam
Copy link
Contributor Author

noklam commented Nov 11, 2022

Trying to bump s3fs but it causes many s3 tests to fail. Suspect it's a moto issue, but seems related to aiobotocore. Potentially also fix kedro-org/kedro-plugins#67 better since we are stuck with a very old moto version and need to handle it in a special way for Python3.10

aio-libs/aiobotocore#755

Should definitely try to upgrade these mocking library, since s3 API have been changing and we are basically testing it with outdated version.

Optionally we may do the same thing to run moto in server mode.
See :

More note:
s3fs use the fixture like this, so it may not be a bad idea to follow them.

@noklam noklam mentioned this pull request Nov 14, 2022
@noklam noklam marked this pull request as draft November 30, 2022 22:10
@noklam
Copy link
Contributor Author

noklam commented Feb 8, 2023

Closed now as this is not urgent. The complexity of upgrading s3fs comes from the unit tests. The functionalities seems working fine but some of the tests are breaking. The comments above lists some potential solution which requires re-writing some of our test with moto

@noklam noklam closed this Feb 8, 2023
@noklam noklam reopened this Aug 8, 2023
@astrojuanlu
Copy link
Member

Should we close this in favour of kedro-org/kedro-plugins#167 ?

@noklam noklam closed this Aug 31, 2023
@noklam noklam deleted the fix/bump-s3fs-version branch August 31, 2023 08:51
@noklam
Copy link
Contributor Author

noklam commented Aug 31, 2023

Closed as we moved dataset out #2017

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.

Bump s3fs version number
2 participants