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

fix(core): accept kwargs in get file #863

Merged
merged 1 commit into from
Mar 18, 2024

Conversation

shcheklein
Copy link
Contributor

Fixes iterative/dvc-s3#80

It is similar to gcsfs and adlfs.

On our end it seems max_concurrency is passed here https://github.com/iterative/dvc-objects/blob/main/src/dvc_objects/fs/generic.py#L210 and since the new version has this attr we pass it now, which most likely leads to this error.

I'm not sure why _gef_file part was not yet implemented. @pmrowla might have a better idea on was it complicated, or less priority. It seems it is the natural next step to do so.

Fixes iterative/dvc-s3#80

It is similar to `gcsfs` and `adlfs`.

On our end it seems `max_concurrency` is passed here https://github.com/iterative/dvc-objects/blob/main/src/dvc_objects/fs/generic.py#L210 and since the new version has this attr we pass it now, which most likely leads to this error.

I'm not sure why `_gef_file` part was not yet implemented. @pmrowla might have a better idea on was it complicated, or less priority. It seems it is the natural next step to do so.
@pmrowla
Copy link
Contributor

pmrowla commented Mar 17, 2024

For reference, max_concurrency for _get_file was not implemented since re-assembling out-of-order chunks is more complex than the _put_file case (where it is handled by the server when using multipart uploads)

@martindurant martindurant merged commit 5cf759d into fsspec:main Mar 18, 2024
21 checks passed
@skshetry
Copy link

@martindurant, do you mind creating a hotfix release?

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.

Possible issue with latest s3fs (2024.3.0): max_concurrency kwarg
4 participants