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

prepare temporary directory when fget_object #1309

Merged
merged 1 commit into from
Sep 24, 2023

Conversation

Laisky
Copy link
Contributor

@Laisky Laisky commented Sep 19, 2023

fget_object got exception when download temporary file in a non-exists directory.

Traceback (most recent call last):
  File "/home/laisky/repo/laisky/ramjet/ramjet/tasks/gptchat/utils.py", line 98, in wrapper
    return await func(self, *args, **kwargs)
  File "/home/laisky/repo/laisky/ramjet/ramjet/tasks/gptchat/utils.py", line 72, in wrapper
    return await func(self, userinfo, *args, **kwargs)
  File "/home/laisky/repo/laisky/ramjet/ramjet/tasks/gptchat/router.py", line 798, in post
    return await ioloop.run_in_executor(
  File "/home/laisky/.pyenv/versions/3.10.0/lib/python3.10/concurrent/futures/thread.py", line 52, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/home/laisky/repo/laisky/ramjet/ramjet/tasks/gptchat/router.py", line 900, in build_chatbot
    self._build_user_chatbot(
  File "/home/laisky/repo/laisky/ramjet/ramjet/tasks/gptchat/router.py", line 926, in _build_user_chatbot
    index = self.load_datasets(
  File "/home/laisky/repo/laisky/ramjet/ramjet/tasks/gptchat/router.py", line 979, in load_datasets
    s3cli.fget_object(
  File "/home/laisky/repo/laisky/ramjet/venv/lib/python3.10/site-packages/minio/api.py", line 1064, in fget_object
    with open(tmp_file_path, "wb") as tmp_file:
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/tmplk7ct4to/dataset.index.W/a4dcdf3e0b5d5ffaa18462a6da92e724.part.minio'

minio/api.py Outdated
@@ -1044,6 +1044,7 @@ def fget_object(self, bucket_name, object_name, file_path,

# Write to a temporary file "file_path.part.minio" before saving.
tmp_file_path = tmp_file_path or f"{file_path}.{stat.etag}.part.minio"
makedirs(os.path.dirname(tmp_file_path)) # prepare temporary file's directory
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is a good idea SDK to create entire directory tree. It is user's responsibility to have correct file_path. BTW this is not temporary directory rather minio-py creates temporary file under the directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file_path is correct and exists, the problem is {stat.etag} contains a /, that make it become a sub-directory that doesn't exists.

CleanShot 2023-09-19 at 09 49 53@2x

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Send a fix to sanitize etag here using helpers.queryencode()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@balamurugana
Copy link
Member

Fix CI failure

if `stat.etag` contains `/`, will cause exception that `fget_object` try
to write to a non-exists directory
@Laisky
Copy link
Contributor Author

Laisky commented Sep 19, 2023

Fix CI failure

done

@harshavardhana harshavardhana merged commit 2ed4e89 into minio:master Sep 24, 2023
15 checks passed
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.

3 participants