From 6ee51fa536f1f9a25f92718503535ebc4280f23b Mon Sep 17 00:00:00 2001 From: Johannes Wagner Date: Wed, 26 Apr 2023 15:50:57 +0200 Subject: [PATCH 01/14] store authentication as variables --- audbackend/core/artifactory.py | 53 ++++++++++++++++++++++++++++++++++ setup.cfg | 2 +- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/audbackend/core/artifactory.py b/audbackend/core/artifactory.py index dac1969d..cc4c5b7a 100644 --- a/audbackend/core/artifactory.py +++ b/audbackend/core/artifactory.py @@ -1,5 +1,7 @@ import os +import typing +import artifactory import dohq_artifactory import audfactory @@ -8,6 +10,56 @@ from audbackend.core.backend import Backend +def _authentication(host) -> typing.Tuple[str, str]: + """Look for username and API key. + + It first looks for the two environment variables + ``ARTIFACTORY_USERNAME`` and + ``ARTIFACTORY_API_KEY``. + + If some of them or both are missing, + it tries to extract them from the + :file:`~/.artifactory_python.cfg` config file. + For that it removes ``http://`` or ``https://`` + from the beginning of ``url`` + and everything that comes after ``/artifactory``. + E.g. for ``https://audeering.jfrog.io/artifactory/data-public/emodb`` + it will look for an entry in the config file under + ``[audeering.jfrog.io/artifactory]``. + + If it cannot find the config file + or a matching entry in the config file + it will set the username to ``anonymous`` + and the API key to an empty string. + If your Artifactory server is configured + to allow anonymous users + you will be able to access the server this way. + + Args: + host: host address, + e.g. https://audeering.jfrog.io/artifactory + + Returns: + username and API key + + """ + username = os.getenv('ARTIFACTORY_USERNAME', None) + apikey = os.getenv('ARTIFACTORY_API_KEY', None) + if apikey is None or username is None: # pragma: no cover + if host.startswith('http://'): + host = host[7:] + elif host.startswith('https://'): + host = host[8:] + config_entry = artifactory.get_global_config_entry(host) + if config_entry is None: + username = 'anonymous' + apikey = '' + else: + username = config_entry['username'] + apikey = config_entry['password'] + return username, apikey + + class Artifactory(Backend): r"""Backend for Artifactory. @@ -23,6 +75,7 @@ def __init__( ): super().__init__(host, repository) + self.username, self.apikey = _authentication(host) self._artifactory = audfactory.path(self.host) self._repo = self._artifactory.find_repository_local(self.repository) diff --git a/setup.cfg b/setup.cfg index 15ac113e..d568a523 100644 --- a/setup.cfg +++ b/setup.cfg @@ -29,7 +29,7 @@ classifiers = packages = find: install_requires = audeer >=1.19.0 - audfactory >=1.0.12 + dohq-artifactory >=0.8.1 setup_requires = setuptools_scm From b985088716a04fbf7f488cfe818bbfca8dad782f Mon Sep 17 00:00:00 2001 From: Johannes Wagner Date: Wed, 26 Apr 2023 16:12:43 +0200 Subject: [PATCH 02/14] remove audfactory.path() --- audbackend/core/artifactory.py | 75 +++++++++++++++++++++++++--------- tests/conftest.py | 17 -------- tests/test_artifactory.py | 5 ++- 3 files changed, 59 insertions(+), 38 deletions(-) diff --git a/audbackend/core/artifactory.py b/audbackend/core/artifactory.py index cc4c5b7a..fb12d369 100644 --- a/audbackend/core/artifactory.py +++ b/audbackend/core/artifactory.py @@ -10,6 +10,18 @@ from audbackend.core.backend import Backend +def _artifactory_path( + path, + username, + apikey, +) -> artifactory.ArtifactoryPath: + r"""Authenticate at Artifactory and get path object.""" + return artifactory.ArtifactoryPath( + path, + auth=(username, apikey), + ) + + def _authentication(host) -> typing.Tuple[str, str]: """Look for username and API key. @@ -22,8 +34,8 @@ def _authentication(host) -> typing.Tuple[str, str]: :file:`~/.artifactory_python.cfg` config file. For that it removes ``http://`` or ``https://`` from the beginning of ``url`` - and everything that comes after ``/artifactory``. - E.g. for ``https://audeering.jfrog.io/artifactory/data-public/emodb`` + and ``/`` from the end. + E.g. for ``https://audeering.jfrog.io/artifactory/` it will look for an entry in the config file under ``[audeering.jfrog.io/artifactory]``. @@ -36,8 +48,7 @@ def _authentication(host) -> typing.Tuple[str, str]: you will be able to access the server this way. Args: - host: host address, - e.g. https://audeering.jfrog.io/artifactory + host: host address Returns: username and API key @@ -50,13 +61,19 @@ def _authentication(host) -> typing.Tuple[str, str]: host = host[7:] elif host.startswith('https://'): host = host[8:] + if host.endswith('/'): + host = host[:-1] config_entry = artifactory.get_global_config_entry(host) if config_entry is None: - username = 'anonymous' - apikey = '' + if username is None: + username = 'anonymous' + if apikey is None: + apikey = '' else: - username = config_entry['username'] - apikey = config_entry['password'] + if username is None: + username = config_entry['username'] + if apikey is None: + apikey = config_entry['password'] return username, apikey @@ -75,9 +92,13 @@ def __init__( ): super().__init__(host, repository) - self.username, self.apikey = _authentication(host) - self._artifactory = audfactory.path(self.host) - self._repo = self._artifactory.find_repository_local(self.repository) + self._username, self._apikey = _authentication(host) + path = _artifactory_path( + self.host, + self._username, + self._apikey, + ) + self._repo = path.find_repository_local(self.repository) def _access( self, @@ -93,7 +114,7 @@ def _checksum( ) -> str: r"""MD5 checksum of file on backend.""" path = self._path(path, version) - return audfactory.checksum(path) + return audfactory.checksum(str(path)) def _collapse( self, @@ -117,8 +138,13 @@ def _create( if self._repo is not None: utils.raise_file_exists_error(str(self._repo.path)) + path = _artifactory_path( + self.host, + self._username, + self._apikey, + ) self._repo = dohq_artifactory.RepositoryLocal( - self._artifactory, + path, self.repository, package_type=dohq_artifactory.RepositoryLocal.GENERIC, ) @@ -137,7 +163,6 @@ def _exists( ) -> bool: r"""Check if file exists on backend.""" path = self._path(path, version) - path = audfactory.path(path) return path.exists() def _expand( @@ -166,7 +191,7 @@ def _get_file( ): r"""Get file from backend.""" src_path = self._path(src_path, version) - audfactory.download(src_path, dst_path, verbose=verbose) + audfactory.download(str(src_path), dst_path, verbose=verbose) def _ls( self, @@ -177,7 +202,11 @@ def _ls( if path.endswith('/'): # find files under sub-path path = self._expand(path) - path = audfactory.path(path) + path = _artifactory_path( + path, + self._username, + self._apikey, + ) if not path.exists(): utils.raise_file_not_found_error(str(path)) paths = [str(x) for x in path.glob("**/*") if x.is_file()] @@ -190,7 +219,8 @@ def _ls( vs = [os.path.basename(str(f)) for f in root if f.is_dir] # filter out other files with same root and version - paths = [self._path(path, v) for v in vs if self._exists(path, v)] + paths = [str(self._path(path, v)) + for v in vs if self._exists(path, v)] if not paths: utils.raise_file_not_found_error(path) @@ -219,7 +249,7 @@ def _path( self, path: str, version: str, - ) -> str: + ) -> artifactory.ArtifactoryPath: r"""Convert to backend path. / @@ -230,6 +260,11 @@ def _path( root, name = self.split(path) root = self._expand(root) path = f'{root}/{version}/{name}' + path = _artifactory_path( + path, + self._username, + self._apikey, + ) return path def _put_file( @@ -241,7 +276,7 @@ def _put_file( ): r"""Put file to backend.""" dst_path = self._path(dst_path, version) - audfactory.deploy(src_path, dst_path, verbose=verbose) + audfactory.deploy(src_path, str(dst_path), verbose=verbose) def _remove_file( self, @@ -250,4 +285,4 @@ def _remove_file( ): r"""Remove file from backend.""" path = self._path(path, version) - audfactory.path(path).unlink() + path.unlink() diff --git a/tests/conftest.py b/tests/conftest.py index 80e62b24..e1d887b2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -65,20 +65,3 @@ def cleanup_coverage(): ) for file in audeer.list_file_names(path): os.remove(file) - - -@pytest.fixture(scope='function', autouse=False) -def no_artifactory_access_rights(): - current_username = os.environ.get('ARTIFACTORY_USERNAME', False) - current_api_key = os.environ.get('ARTIFACTORY_API_KEY', False) - os.environ['ARTIFACTORY_USERNAME'] = 'non-existing-user' - os.environ['ARTIFACTORY_API_KEY'] = 'non-existing-password' - yield - if current_username: - os.environ["ARTIFACTORY_USERNAME"] = current_username - else: - del os.environ['ARTIFACTORY_USERNAME'] - if current_api_key: - os.environ['ARTIFACTORY_API_KEY'] = current_api_key - else: - del os.environ['ARTIFACTORY_API_KEY'] diff --git a/tests/test_artifactory.py b/tests/test_artifactory.py index 6569f975..747b6619 100644 --- a/tests/test_artifactory.py +++ b/tests/test_artifactory.py @@ -9,7 +9,10 @@ ['artifactory'], indirect=True, ) -def test_errors(tmpdir, backend, no_artifactory_access_rights): +def test_errors(tmpdir, backend): + + backend._username = 'non-existing' + backend._apikey = 'non-existing' local_file = audeer.touch( audeer.path(tmpdir, 'file.txt') From 4047981d0e002abf3ffe01bbc7282ecddb02de8a Mon Sep 17 00:00:00 2001 From: Johannes Wagner Date: Wed, 26 Apr 2023 16:16:14 +0200 Subject: [PATCH 03/14] remove audfactory.checksum() --- audbackend/core/artifactory.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/audbackend/core/artifactory.py b/audbackend/core/artifactory.py index fb12d369..e1ce37da 100644 --- a/audbackend/core/artifactory.py +++ b/audbackend/core/artifactory.py @@ -114,7 +114,8 @@ def _checksum( ) -> str: r"""MD5 checksum of file on backend.""" path = self._path(path, version) - return audfactory.checksum(str(path)) + checksum = artifactory.ArtifactoryPath.stat(path).md5 + return checksum def _collapse( self, @@ -215,7 +216,11 @@ def _ls( root, _ = self.split(path) root = self._expand(root) - root = audfactory.path(root) + root = _artifactory_path( + root, + self._username, + self._apikey, + ) vs = [os.path.basename(str(f)) for f in root if f.is_dir] # filter out other files with same root and version From fb1bf25e0b9f8d5f321bcf19b21f5fce560714b2 Mon Sep 17 00:00:00 2001 From: Johannes Wagner Date: Wed, 26 Apr 2023 16:23:17 +0200 Subject: [PATCH 04/14] remove audfactory.download() --- audbackend/core/artifactory.py | 56 +++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/audbackend/core/artifactory.py b/audbackend/core/artifactory.py index e1ce37da..50803a41 100644 --- a/audbackend/core/artifactory.py +++ b/audbackend/core/artifactory.py @@ -4,6 +4,7 @@ import artifactory import dohq_artifactory +import audeer import audfactory from audbackend.core import utils @@ -77,6 +78,59 @@ def _authentication(host) -> typing.Tuple[str, str]: return username, apikey +def _download( + src_path: artifactory.ArtifactoryPath, + dst_path: str, + *, + chunk: int = 4 * 1024, + verbose=False, +) -> str: + r"""Download an artifact. + + Args: + src_path: artifact URL + dst_path: path to store the artifact + chunk: amount of data read at once during the download + verbose: show information on the download process + + Returns: + path to local artifact + + Raises: + RuntimeError: if artifact cannot be found, + or you don't have access rights to the artifact + + """ + src_size = artifactory.ArtifactoryPath.stat(src_path).size + + with audeer.progress_bar(total=src_size, disable=not verbose) as pbar: + desc = audeer.format_display_message( + 'Download {}'.format(os.path.basename(str(src_path))), + pbar=True, + ) + pbar.set_description_str(desc) + pbar.refresh() + + try: + dst_size = 0 + with src_path.open() as src_fp: + with open(dst_path, 'wb') as dst_fp: + while src_size > dst_size: + data = src_fp.read(chunk) + n_data = len(data) + if n_data > 0: + dst_fp.write(data) + dst_size += n_data + pbar.update(n_data) + except (KeyboardInterrupt, Exception): + # Clean up broken artifact files + if os.path.exists(dst_path): + os.remove(dst_path) # pragma: no cover + raise + + return dst_path + + class Artifactory(Backend): r"""Backend for Artifactory. @@ -192,7 +246,7 @@ def _get_file( ): r"""Get file from backend.""" src_path = self._path(src_path, version) - audfactory.download(str(src_path), dst_path, verbose=verbose) + _download(src_path, dst_path, verbose=verbose) def _ls( self, From 3496cb856dcd58aeec651c661dc8d5de1eb55e74 Mon Sep 17 00:00:00 2001 From: Johannes Wagner Date: Wed, 26 Apr 2023 16:29:37 +0200 Subject: [PATCH 05/14] remove audfactory.deploy() --- audbackend/core/artifactory.py | 51 ++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/audbackend/core/artifactory.py b/audbackend/core/artifactory.py index 50803a41..09288859 100644 --- a/audbackend/core/artifactory.py +++ b/audbackend/core/artifactory.py @@ -78,6 +78,42 @@ def _authentication(host) -> typing.Tuple[str, str]: return username, apikey +def _deploy( + src_path: str, + dst_path: artifactory.ArtifactoryPath, + *, + verbose: bool = False, +): + r"""Deploy local file as an artifact. + + Args: + src_path: local file path + dst_path: path on Artifactory + verbose: show information on the upload process + + """ + if verbose: # pragma: no cover + desc = audeer.format_display_message( + f'Deploy {src_path}', + pbar=False, + ) + print(desc, end='\r') + + md5 = utils.md5(src_path) + + if not dst_path.parent.exists(): + dst_path.parent.mkdir() + with open(src_path, "rb") as fobj: + dst_path.deploy( + fobj, + md5=md5, + ) + + if verbose: # pragma: no cover + # Final clearing of progress line + print(audeer.format_display_message(' ', pbar=False), end='\r') + + def _download( src_path: artifactory.ArtifactoryPath, dst_path: str, @@ -88,18 +124,11 @@ def _download( r"""Download an artifact. Args: - src_path: artifact URL - dst_path: path to store the artifact + src_path: local file path + dst_path: path on Artifactory chunk: amount of data read at once during the download verbose: show information on the download process - Returns: - path to local artifact - - Raises: - RuntimeError: if artifact cannot be found, - or you don't have access rights to the artifact - """ src_size = artifactory.ArtifactoryPath.stat(src_path).size @@ -128,8 +157,6 @@ def _download( os.remove(dst_path) # pragma: no cover raise - return dst_path - class Artifactory(Backend): r"""Backend for Artifactory. @@ -335,7 +362,7 @@ def _put_file( ): r"""Put file to backend.""" dst_path = self._path(dst_path, version) - audfactory.deploy(src_path, str(dst_path), verbose=verbose) + _deploy(src_path, dst_path, verbose=verbose) def _remove_file( self, From 3f98cd218483c041204f94030c56be88997e1061 Mon Sep 17 00:00:00 2001 From: Johannes Wagner Date: Wed, 26 Apr 2023 16:29:56 +0200 Subject: [PATCH 06/14] remove import of audfactory --- audbackend/core/artifactory.py | 1 - 1 file changed, 1 deletion(-) diff --git a/audbackend/core/artifactory.py b/audbackend/core/artifactory.py index 09288859..03dd773f 100644 --- a/audbackend/core/artifactory.py +++ b/audbackend/core/artifactory.py @@ -5,7 +5,6 @@ import dohq_artifactory import audeer -import audfactory from audbackend.core import utils from audbackend.core.backend import Backend From 2567a33ea6d6fcd222aba4a37a847ac69457944e Mon Sep 17 00:00:00 2001 From: Johannes Wagner Date: Wed, 26 Apr 2023 16:36:42 +0200 Subject: [PATCH 07/14] clean up --- audbackend/core/artifactory.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/audbackend/core/artifactory.py b/audbackend/core/artifactory.py index 03dd773f..4fdd268d 100644 --- a/audbackend/core/artifactory.py +++ b/audbackend/core/artifactory.py @@ -102,11 +102,8 @@ def _deploy( if not dst_path.parent.exists(): dst_path.parent.mkdir() - with open(src_path, "rb") as fobj: - dst_path.deploy( - fobj, - md5=md5, - ) + with open(src_path, 'rb') as fd: + dst_path.deploy(fd, md5=md5) if verbose: # pragma: no cover # Final clearing of progress line @@ -119,7 +116,7 @@ def _download( *, chunk: int = 4 * 1024, verbose=False, -) -> str: +): r"""Download an artifact. Args: @@ -150,10 +147,10 @@ def _download( dst_fp.write(data) dst_size += n_data pbar.update(n_data) - except (KeyboardInterrupt, Exception): + except (KeyboardInterrupt, Exception): # pragma: no cover # Clean up broken artifact files if os.path.exists(dst_path): - os.remove(dst_path) # pragma: no cover + os.remove(dst_path) raise From b115e031693b7507f526ef3a18113aed863e5808 Mon Sep 17 00:00:00 2001 From: Johannes Wagner Date: Wed, 26 Apr 2023 22:07:42 +0200 Subject: [PATCH 08/14] TST: full cover authentication --- audbackend/core/artifactory.py | 50 ++++++++++++++++------------------ tests/test_artifactory.py | 38 ++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 26 deletions(-) diff --git a/audbackend/core/artifactory.py b/audbackend/core/artifactory.py index 4fdd268d..e56af8a6 100644 --- a/audbackend/core/artifactory.py +++ b/audbackend/core/artifactory.py @@ -56,24 +56,27 @@ def _authentication(host) -> typing.Tuple[str, str]: """ username = os.getenv('ARTIFACTORY_USERNAME', None) apikey = os.getenv('ARTIFACTORY_API_KEY', None) - if apikey is None or username is None: # pragma: no cover + + if apikey is None or username is None: + if host.startswith('http://'): host = host[7:] elif host.startswith('https://'): host = host[8:] if host.endswith('/'): host = host[:-1] + config_entry = artifactory.get_global_config_entry(host) - if config_entry is None: - if username is None: - username = 'anonymous' - if apikey is None: - apikey = '' - else: + + if config_entry is not None: if username is None: username = config_entry['username'] if apikey is None: apikey = config_entry['password'] + + username = username if username is not None else 'anonymous' + apikey = apikey if apikey is not None else '' + return username, apikey @@ -98,15 +101,15 @@ def _deploy( ) print(desc, end='\r') - md5 = utils.md5(src_path) - if not dst_path.parent.exists(): dst_path.parent.mkdir() + + md5 = utils.md5(src_path) with open(src_path, 'rb') as fd: dst_path.deploy(fd, md5=md5) if verbose: # pragma: no cover - # Final clearing of progress line + # Clear progress line print(audeer.format_display_message(' ', pbar=False), end='\r') @@ -129,6 +132,7 @@ def _download( src_size = artifactory.ArtifactoryPath.stat(src_path).size with audeer.progress_bar(total=src_size, disable=not verbose) as pbar: + desc = audeer.format_display_message( 'Download {}'.format(os.path.basename(str(src_path))), pbar=True, @@ -136,22 +140,16 @@ def _download( pbar.set_description_str(desc) pbar.refresh() - try: - dst_size = 0 - with src_path.open() as src_fp: - with open(dst_path, 'wb') as dst_fp: - while src_size > dst_size: - data = src_fp.read(chunk) - n_data = len(data) - if n_data > 0: - dst_fp.write(data) - dst_size += n_data - pbar.update(n_data) - except (KeyboardInterrupt, Exception): # pragma: no cover - # Clean up broken artifact files - if os.path.exists(dst_path): - os.remove(dst_path) - raise + dst_size = 0 + with src_path.open() as src_fp: + with open(dst_path, 'wb') as dst_fp: + while src_size > dst_size: + data = src_fp.read(chunk) + n_data = len(data) + if n_data > 0: + dst_fp.write(data) + dst_size += n_data + pbar.update(n_data) class Artifactory(Backend): diff --git a/tests/test_artifactory.py b/tests/test_artifactory.py index 747b6619..fd45fab2 100644 --- a/tests/test_artifactory.py +++ b/tests/test_artifactory.py @@ -1,9 +1,47 @@ +import os + +import dohq_artifactory import pytest import audbackend import audeer +def test_authentication(): + + # read credentials from config file + + for host in [ + 'https://audeering.jfrog.io/artifactory', + 'http://audeering.jfrog.io/artifactory', + 'http://audeering.jfrog.io/artifactory/', + ]: + backend = audbackend.Artifactory( + host, + 'repository', + ) + assert backend._username == 'audeering-unittest' + assert backend._apikey != '' + + # read credentials from environment variables + + for env in ['ARTIFACTORY_USERNAME', 'ARTIFACTORY_USERNAME']: + + tmp = os.environ.get(env, False) + os.environ[env] = 'bad' + + with pytest.raises(dohq_artifactory.exception.ArtifactoryException): + audbackend.Artifactory( + 'https://audeering.jfrog.io/artifactory', + 'repository', + ) + + if tmp: + os.environ[env] = tmp + else: + del os.environ[env] + + @pytest.mark.parametrize( 'backend', ['artifactory'], From f08fb0b01ca2066d82b3c44408d2eef3fb75b38d Mon Sep 17 00:00:00 2001 From: Johannes Wagner Date: Wed, 26 Apr 2023 22:28:26 +0200 Subject: [PATCH 09/14] TST: remove authentication test --- audbackend/core/artifactory.py | 20 +++--------------- tests/test_artifactory.py | 38 ---------------------------------- 2 files changed, 3 insertions(+), 55 deletions(-) diff --git a/audbackend/core/artifactory.py b/audbackend/core/artifactory.py index e56af8a6..887d00be 100644 --- a/audbackend/core/artifactory.py +++ b/audbackend/core/artifactory.py @@ -25,19 +25,12 @@ def _artifactory_path( def _authentication(host) -> typing.Tuple[str, str]: """Look for username and API key. - It first looks for the two environment variables + Looks for the two environment variables ``ARTIFACTORY_USERNAME`` and ``ARTIFACTORY_API_KEY``. - If some of them or both are missing, - it tries to extract them from the + Missing values are extracted them from the :file:`~/.artifactory_python.cfg` config file. - For that it removes ``http://`` or ``https://`` - from the beginning of ``url`` - and ``/`` from the end. - E.g. for ``https://audeering.jfrog.io/artifactory/` - it will look for an entry in the config file under - ``[audeering.jfrog.io/artifactory]``. If it cannot find the config file or a matching entry in the config file @@ -57,14 +50,7 @@ def _authentication(host) -> typing.Tuple[str, str]: username = os.getenv('ARTIFACTORY_USERNAME', None) apikey = os.getenv('ARTIFACTORY_API_KEY', None) - if apikey is None or username is None: - - if host.startswith('http://'): - host = host[7:] - elif host.startswith('https://'): - host = host[8:] - if host.endswith('/'): - host = host[:-1] + if apikey is None or username is None: # pragma: no cover config_entry = artifactory.get_global_config_entry(host) diff --git a/tests/test_artifactory.py b/tests/test_artifactory.py index fd45fab2..747b6619 100644 --- a/tests/test_artifactory.py +++ b/tests/test_artifactory.py @@ -1,47 +1,9 @@ -import os - -import dohq_artifactory import pytest import audbackend import audeer -def test_authentication(): - - # read credentials from config file - - for host in [ - 'https://audeering.jfrog.io/artifactory', - 'http://audeering.jfrog.io/artifactory', - 'http://audeering.jfrog.io/artifactory/', - ]: - backend = audbackend.Artifactory( - host, - 'repository', - ) - assert backend._username == 'audeering-unittest' - assert backend._apikey != '' - - # read credentials from environment variables - - for env in ['ARTIFACTORY_USERNAME', 'ARTIFACTORY_USERNAME']: - - tmp = os.environ.get(env, False) - os.environ[env] = 'bad' - - with pytest.raises(dohq_artifactory.exception.ArtifactoryException): - audbackend.Artifactory( - 'https://audeering.jfrog.io/artifactory', - 'repository', - ) - - if tmp: - os.environ[env] = tmp - else: - del os.environ[env] - - @pytest.mark.parametrize( 'backend', ['artifactory'], From 266d2af63351e354f232f8197a17eb32574aacd5 Mon Sep 17 00:00:00 2001 From: Johannes Wagner Date: Thu, 27 Apr 2023 09:43:02 +0200 Subject: [PATCH 10/14] TST: test authenthication --- audbackend/core/artifactory.py | 99 ++++++++++++++++------------------ tests/test_artifactory.py | 58 +++++++++++++++++++- 2 files changed, 103 insertions(+), 54 deletions(-) diff --git a/audbackend/core/artifactory.py b/audbackend/core/artifactory.py index 887d00be..9b3694da 100644 --- a/audbackend/core/artifactory.py +++ b/audbackend/core/artifactory.py @@ -16,6 +16,7 @@ def _artifactory_path( apikey, ) -> artifactory.ArtifactoryPath: r"""Authenticate at Artifactory and get path object.""" + return artifactory.ArtifactoryPath( path, auth=(username, apikey), @@ -23,47 +24,32 @@ def _artifactory_path( def _authentication(host) -> typing.Tuple[str, str]: - """Look for username and API key. - - Looks for the two environment variables - ``ARTIFACTORY_USERNAME`` and - ``ARTIFACTORY_API_KEY``. - - Missing values are extracted them from the - :file:`~/.artifactory_python.cfg` config file. - - If it cannot find the config file - or a matching entry in the config file - it will set the username to ``anonymous`` - and the API key to an empty string. - If your Artifactory server is configured - to allow anonymous users - you will be able to access the server this way. - - Args: - host: host address - - Returns: - username and API key + """Look for username and API key.""" - """ username = os.getenv('ARTIFACTORY_USERNAME', None) - apikey = os.getenv('ARTIFACTORY_API_KEY', None) + api_key = os.getenv('ARTIFACTORY_API_KEY', None) + config_file = os.getenv( + 'ARTIFACTORY_CONFIG_FILE', + artifactory.default_config_path, + ) - if apikey is None or username is None: # pragma: no cover + if api_key is None or username is None: - config_entry = artifactory.get_global_config_entry(host) + config = artifactory.read_config(config_file) + config_entry = artifactory.get_config_entry(config, host) if config_entry is not None: if username is None: username = config_entry['username'] - if apikey is None: - apikey = config_entry['password'] + if api_key is None: + api_key = config_entry['password'] - username = username if username is not None else 'anonymous' - apikey = apikey if apikey is not None else '' + if username is None: + username = 'anonymous' + if api_key is None: + api_key = '' - return username, apikey + return username, api_key def _deploy( @@ -72,14 +58,8 @@ def _deploy( *, verbose: bool = False, ): - r"""Deploy local file as an artifact. - - Args: - src_path: local file path - dst_path: path on Artifactory - verbose: show information on the upload process + r"""Deploy local file as an artifact.""" - """ if verbose: # pragma: no cover desc = audeer.format_display_message( f'Deploy {src_path}', @@ -106,15 +86,8 @@ def _download( chunk: int = 4 * 1024, verbose=False, ): - r"""Download an artifact. + r"""Download an artifact.""" - Args: - src_path: local file path - dst_path: path on Artifactory - chunk: amount of data read at once during the download - verbose: show information on the download process - - """ src_size = artifactory.ArtifactoryPath.stat(src_path).size with audeer.progress_bar(total=src_size, disable=not verbose) as pbar: @@ -141,11 +114,31 @@ def _download( class Artifactory(Backend): r"""Backend for Artifactory. + Looks for the two environment variables + ``ARTIFACTORY_USERNAME`` and + ``ARTIFACTORY_API_KEY``. + Otherwise, + tries to extract missing values + from a global `config file`_. + The default path of the config file + (:file:`~/.artifactory_python.cfg`) + can be overwritten with the environment variable + ``ARTIFACTORY_CONFIG_FILE``. + If no config file exists + or if it does not contain an + entry for the ``host``, + the username is set to ``'anonymous'`` + and the API key to an empty string. + In that case the ``host`` + should support anonymous access. + Args: host: host address repository: repository name - """ + .. _`config file`: https://github.com/devopshq/artifactory#global-configuration-file + + """ # noqa: E501 def __init__( self, host, @@ -153,11 +146,11 @@ def __init__( ): super().__init__(host, repository) - self._username, self._apikey = _authentication(host) + self._username, self._api_key = _authentication(host) path = _artifactory_path( self.host, self._username, - self._apikey, + self._api_key, ) self._repo = path.find_repository_local(self.repository) @@ -203,7 +196,7 @@ def _create( path = _artifactory_path( self.host, self._username, - self._apikey, + self._api_key, ) self._repo = dohq_artifactory.RepositoryLocal( path, @@ -267,7 +260,7 @@ def _ls( path = _artifactory_path( path, self._username, - self._apikey, + self._api_key, ) if not path.exists(): utils.raise_file_not_found_error(str(path)) @@ -280,7 +273,7 @@ def _ls( root = _artifactory_path( root, self._username, - self._apikey, + self._api_key, ) vs = [os.path.basename(str(f)) for f in root if f.is_dir] @@ -329,7 +322,7 @@ def _path( path = _artifactory_path( path, self._username, - self._apikey, + self._api_key, ) return path diff --git a/tests/test_artifactory.py b/tests/test_artifactory.py index 747b6619..45d7a028 100644 --- a/tests/test_artifactory.py +++ b/tests/test_artifactory.py @@ -1,9 +1,65 @@ +import os + +import dohq_artifactory import pytest import audbackend import audeer +@pytest.fixture(scope='function', autouse=False) +def hide_credentials(): + + defaults = {} + + for key in [ + 'ARTIFACTORY_USERNAME', + 'ARTIFACTORY_API_KEY', + 'ARTIFACTORY_CONFIG_FILE', + ]: + defaults[key] = os.environ.get(key, None) + + for key, value in defaults.items(): + if value is not None: + del os.environ[key] + + yield + + for key, value in defaults.items(): + if value is not None: + os.environ[key] = value + elif key in os.environ: + del os.environ[key] + + +def test_authentication(tmpdir, hosts, hide_credentials): + + host = hosts['artifactory'] + + # create empty config file + + config_path = audeer.path(tmpdir, 'config.cfg') + os.environ['ARTIFACTORY_CONFIG_FILE'] = audeer.touch(config_path) + + # default credentials + + backend = audbackend.Artifactory(host, 'repository') + assert backend._username == 'anonymous' + assert backend._api_key == '' + + # read from config file + + username = 'bad' + api_key = 'bad' + with open(config_path, 'w') as fp: + fp.write(f'[{host}]\n') + fp.write(f'username = {username}]\n') + fp.write(f'password = {api_key}]\n') + + with pytest.raises(dohq_artifactory.exception.ArtifactoryException): + audbackend.Artifactory(host, 'repository') + + @pytest.mark.parametrize( 'backend', ['artifactory'], @@ -12,7 +68,7 @@ def test_errors(tmpdir, backend): backend._username = 'non-existing' - backend._apikey = 'non-existing' + backend._api_key = 'non-existing' local_file = audeer.touch( audeer.path(tmpdir, 'file.txt') From 8d7a55cfb73f1e082cc054c52165d05ddd540363 Mon Sep 17 00:00:00 2001 From: Johannes Wagner Date: Thu, 27 Apr 2023 09:55:25 +0200 Subject: [PATCH 11/14] DOC: fix link to artifactory docu --- audbackend/core/artifactory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/audbackend/core/artifactory.py b/audbackend/core/artifactory.py index 9b3694da..a2b7289f 100644 --- a/audbackend/core/artifactory.py +++ b/audbackend/core/artifactory.py @@ -136,7 +136,7 @@ class Artifactory(Backend): host: host address repository: repository name - .. _`config file`: https://github.com/devopshq/artifactory#global-configuration-file + .. _`config file`: https://devopshq.github.io/artifactory/#global-configuration-file """ # noqa: E501 def __init__( From e18348daed52047db1741198162f0aaedbf83376 Mon Sep 17 00:00:00 2001 From: Johannes Wagner Date: Thu, 27 Apr 2023 11:21:34 +0200 Subject: [PATCH 12/14] Update audbackend/core/artifactory.py Co-authored-by: Hagen Wierstorf --- audbackend/core/artifactory.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/audbackend/core/artifactory.py b/audbackend/core/artifactory.py index a2b7289f..678f3bfe 100644 --- a/audbackend/core/artifactory.py +++ b/audbackend/core/artifactory.py @@ -40,9 +40,9 @@ def _authentication(host) -> typing.Tuple[str, str]: if config_entry is not None: if username is None: - username = config_entry['username'] + username = config_entry.get('username', None) if api_key is None: - api_key = config_entry['password'] + api_key = config_entry.get('password', None) if username is None: username = 'anonymous' From c1ebf046ea478a8d3242981a83a30045fcdc9cec Mon Sep 17 00:00:00 2001 From: Johannes Wagner Date: Thu, 27 Apr 2023 11:23:44 +0200 Subject: [PATCH 13/14] TST: test with empty config entry --- tests/test_artifactory.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/test_artifactory.py b/tests/test_artifactory.py index 45d7a028..67f02a36 100644 --- a/tests/test_artifactory.py +++ b/tests/test_artifactory.py @@ -47,7 +47,16 @@ def test_authentication(tmpdir, hosts, hide_credentials): assert backend._username == 'anonymous' assert backend._api_key == '' - # read from config file + # config file entry without username and password + + with open(config_path, 'w') as fp: + fp.write(f'[{host}]\n') + + backend = audbackend.Artifactory(host, 'repository') + assert backend._username == 'anonymous' + assert backend._api_key == '' + + # config file entry with username and password username = 'bad' api_key = 'bad' From 584fe0fe60282b771e1f5800dfb01dc9951f53a6 Mon Sep 17 00:00:00 2001 From: Johannes Wagner Date: Thu, 27 Apr 2023 11:27:09 +0200 Subject: [PATCH 14/14] Update tests/test_artifactory.py Co-authored-by: Hagen Wierstorf --- tests/test_artifactory.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_artifactory.py b/tests/test_artifactory.py index 67f02a36..aea03daa 100644 --- a/tests/test_artifactory.py +++ b/tests/test_artifactory.py @@ -62,8 +62,8 @@ def test_authentication(tmpdir, hosts, hide_credentials): api_key = 'bad' with open(config_path, 'w') as fp: fp.write(f'[{host}]\n') - fp.write(f'username = {username}]\n') - fp.write(f'password = {api_key}]\n') + fp.write(f'username = {username}\n') + fp.write(f'password = {api_key}\n') with pytest.raises(dohq_artifactory.exception.ArtifactoryException): audbackend.Artifactory(host, 'repository')