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

fget_object raises error when local path has nested directory #31

Closed
KelvinHong opened this issue May 15, 2024 · 3 comments
Closed

fget_object raises error when local path has nested directory #31

KelvinHong opened this issue May 15, 2024 · 3 comments

Comments

@KelvinHong
Copy link
Contributor

The method MockMinioClient.fget_object doesn't work as expected when the file_path argument has a nested folder structure and doesn't exists.

def fget_object(
self,
bucket_name,
object_name,
file_path,
request_headers=None,
sse=None,
version_id=None,
extra_query_params=None,
):
"""
Simulates Minio's 'fget_object' method to download an object from the mock Minio
server and save it to a file.
This mock method retrieves the specified object from the given bucket and writes its
contents to the specified file path. It's a part of the MockMinioClient class that
simulates interactions with a Minio server for testing purposes.
Args:
bucket_name (str): The name of the bucket containing the object to download.
object_name (str): The name of the object to download.
file_path (str): The path to the file where the object's data should be saved.
request_headers (dict, optional): A dictionary of headers to send along with the
request. Defaults to None.
sse (optional): Server-side encryption option. Defaults to None, as it's not used in
the mock.
version_id (str | None, optional): The version ID of the object to download. Defaults to
None.
extra_query_params (dict, optional): Additional query parameters for the request.
Defaults to None.
Raises:
ValueError: If the specified bucket or object doesn't exist in the mock data.
IOError: If there's an issue writing to the specified file path.
Returns:
None: The method writes the object's data to a file and has no return value.
"""
the_object = self.get_object(
bucket_name,
object_name,
version_id=version_id,
request_headers=request_headers,
extra_query_params=extra_query_params,
)
with open(file_path, "wb") as f:
f.write(the_object.data)

Expected behavior:
fget_object should create the parent directory of file_path before attempting to get_object.

Also, this is the implementation from minio-py

https://github.com/minio/minio-py/blob/fd3571aa9b9402f050192ad4c6c087d2806ca9eb/minio/api.py#L1116

My proposed solution:

Add this line before calling self.get_object:

os.makedirs(os.path.dirname(file_path), exist_ok=True)

Let me know what do you think about this! Or is there any reason why makedirs is purposely not called?

@oussjarrousse
Copy link
Owner

oussjarrousse commented May 16, 2024

Thank you for opening the issue, and for the fix suggestion.
I actually identified several other interface issues with fget_object().
Usually I encourage users to submit a pull request. but this time I am fixing it myself because it is a small issue and I am experimenting with a branching and CI/CD pipline.

Here is the pull request: #32

Let me know if you have any comments.

I will merge it and publish the new version over the weekend.

@KelvinHong
Copy link
Contributor Author

I think the fix is fine! Thank you for the quick fix.

@oussjarrousse
Copy link
Owner

I just published a new release v0.4.14
https://pypi.org/project/pytest-minio-mock/0.4.14/
closing this issue!

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

No branches or pull requests

2 participants