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

Fixes KeyError when retrieving empty but existing object from S3 #771

Merged
merged 2 commits into from
Sep 6, 2023

Conversation

Darkheir
Copy link
Contributor

Title

Attempt to fix the KeyError we have when reading an empty file from S3

Motivation

When we try to retrieve an empty file from S3 we may have the following error

...
File "/home/user/.local/lib/python3.8/site-packages/smart_open/s3.py", line 450, in _open_body
    self._position = self._content_length = int(error_response['ActualObjectSize'])
KeyError: 'ActualObjectSize'

This PR catches the KeyError and send a get_object query without any range to retrieve the data as suggested in the issue

Tests

I added a test, but it seems that the Moto lib provide the value ActualObjectSize even for empty objects so the code won't be tested...

@mpenkov
Copy link
Collaborator

mpenkov commented May 31, 2023

Can you please add a test?

@Darkheir
Copy link
Contributor Author

Darkheir commented Jun 1, 2023

A test has been added :)

@Darkheir
Copy link
Contributor Author

Do you need anything more to be done in the PR before merging it ?

@mpenkov
Copy link
Collaborator

mpenkov commented Jun 17, 2023

No, please leave it with me. I'm a little busy with other stuff at the moment, sorry.

@mpenkov mpenkov changed the title fix: Fixes KeyError when retrieving empty file from S3 Fixes KeyError when retrieving empty file from S3 Sep 6, 2023
@mpenkov mpenkov changed the title Fixes KeyError when retrieving empty file from S3 Fixes KeyError when retrieving empty but existing object from S3 Sep 6, 2023
@mpenkov mpenkov merged commit 085b711 into piskvorky:develop Sep 6, 2023
@mpenkov
Copy link
Collaborator

mpenkov commented Sep 6, 2023

Thank you @Darkheir !

beck3905 pushed a commit to beck3905/smart_open that referenced this pull request Sep 6, 2023
…kvorky#771)

* fix: Fixes KeyError when retrieving empty file from S3

* Add test
mpenkov pushed a commit that referenced this pull request Sep 7, 2023
* fix: ignore seek requests to the current position

* fix: adjust test to match new seek behavior

* run seek if it is the first time

* Add required import for example to work (#756)

If a person were to simply copy this code block it would use the built in `open` and would not work. Adding in the correct import makes this block a bit easier for a simple copy paste.

* run tests on py3.11 (#774)

* add type command to ftp (#781)

* Add python 3.11 to setup.py (#775)

* Fixes KeyError when retrieving empty but existing object from S3 (#771)

* fix: Fixes KeyError when retrieving empty file from S3

* Add test

* bugfix: when read size > chunk size, return read size and not chunk size (#767)

* undo formatting

* fix whitespace

* undo formatting

---------

Co-authored-by: Rusty Conover <rusty@conover.me>
Co-authored-by: Christian Jensen <christian@orbik.com>
Co-authored-by: tooptoop4 <33283496+tooptoop4@users.noreply.github.com>
Co-authored-by: Raphaël Cohen <raphael.cohen.utt@gmail.com>
Co-authored-by: Ron Reiter <ron.reiter@gmail.com>
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.

Reading a 0 B file from s3 raises KeyError
2 participants