Skip to content

Add support for owner() #234

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.

Sign up for GitHub

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

Merged
hagenw merged 2 commits into from
Oct 18, 2024
Merged

Add support for owner() #234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64c9abb
Add support for owner()
hagenw Sep 30, 2024
616da64
Be more conservative regarding owner
hagenw Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions audbackend/core/backend/minio.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import configparser
import getpass
import mimetypes
import os
import tempfile
Expand Down Expand Up @@ -213,6 +214,7 @@ def _copy_file(
self.repository,
dst_path,
minio.commonconfig.CopySource(self.repository, src_path),
metadata=_metadata(),
)

def _create(
Expand Down Expand Up @@ -330,9 +332,13 @@ def _owner(
) -> str:
r"""Get owner of file on backend."""
path = self.path(path)
# TODO: owner seems to be empty,
# need to check if we have to manage this ourselves?
owner = self._client.stat_object(self.repository, path).owner_name
# NOTE:
# we use a custom metadata entry to track the owner
# as stats.owner_name is always empty.
owner = ""
stats = self._client.stat_object(self.repository, path)
if "x-amz-meta-owner" in stats.metadata:
owner = stats.metadata["x-amz-meta-owner"]
return owner

def path(
Expand Down Expand Up @@ -376,6 +382,7 @@ def _put_file(
dst_path,
src_path,
content_type=content_type,
metadata=_metadata(),
)

if verbose: # pragma: no cover
Expand All @@ -398,3 +405,13 @@ def _size(
path = self.path(path)
size = self._client.stat_object(self.repository, path).size
return size


def _metadata():

@ChristianGeng ChristianGeng Oct 18, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a reason to maintain _metadata completely separate from the Minio class?
Intuitively one would think of _metadata as a (private) property. On the other hand, the class needs to implement the interface methods from the parent class, and a property might become confusing.

@hagenw hagenw Oct 18, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I tend to implement such helper functions independent of the class. My main reason is that in the class we only have the methods that needed to be added, and it does not get cluttered. But maybe it has also some drawbacks?

"""Dictionary with owner entry.

When uploaded as metadata to MinIO,
it can be accessed under ``stat_object(...).metadata["x-amz-meta-owner"]``.

"""
return {"owner": getpass.getuser()}
5 changes: 0 additions & 5 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,6 @@ def owner(request):
):
host_wo_https = pytest.HOSTS["artifactory"][8:]
owner = backend_cls.get_authentication(host_wo_https)[0]
elif (
hasattr(audbackend.backend, "Minio") and backend_cls == audbackend.backend.Minio
):
# There seems to be a MinIO bug here
owner = None
else:
if os.name == "nt":
owner = "Administrators"
Expand Down