From 03b61a840940ceb12ab3c727ac2660c82417c4f3 Mon Sep 17 00:00:00 2001 From: Matthias Probst Date: Wed, 13 Dec 2023 14:58:26 +0100 Subject: [PATCH] bugfixes --- .github/workflows/tests.yml | 2 +- h5rdmtoolbox/database/zenodo/config.py | 20 ++- h5rdmtoolbox/database/zenodo/deposit.py | 119 ++++++++++-------- .../test_transformations_and_affixes.py | 3 + tests/conventions/test_conventions.py | 4 +- tests/database/test_zenodo.py | 2 +- 6 files changed, 91 insertions(+), 59 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 017feaa9..3f32eab0 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -60,7 +60,7 @@ jobs: - name: Run pytest coverage env: - ZENODO_API_TOKEN: ${{ secrets.ZENODO_API_TOKEN }} + ZENODO_SANDBOX_API_TOKEN: ${{ secrets.ZENODO_SANDBOX_API_TOKEN }} run: pytest --cov --cov-report=xml - name: Upload coverage to Codecov diff --git a/h5rdmtoolbox/database/zenodo/config.py b/h5rdmtoolbox/database/zenodo/config.py index 21f11db4..0a151d89 100644 --- a/h5rdmtoolbox/database/zenodo/config.py +++ b/h5rdmtoolbox/database/zenodo/config.py @@ -1,3 +1,5 @@ +import warnings + import appdirs import configparser import os @@ -22,10 +24,13 @@ def _parse_ini_file(zenodo_ini_filename: Union[str, pathlib.Path]): def get_api_token(sandbox: bool, zenodo_ini_filename: Union[str, pathlib.Path] = None): """Read the Zenodo API token from the config file.""" - env_token = os.environ.get('ZENODO_API_TOKEN', None) + if sandbox: + env_token = os.environ.get('ZENODO_SANDBOX_API_TOKEN', None) + else: + env_token = os.environ.get('ZENODO_API_TOKEN', None) if env_token is not None: env_token = env_token.strip() - logger.debug('Took token from environment variable ZENODO_API_TOKEN.') + logger.debug('Took token from environment variable ZENODO_SANDBOX_API_TOKEN.') return env_token zenodo_ini_filename = _parse_ini_file(zenodo_ini_filename) config = configparser.ConfigParser() @@ -35,9 +40,14 @@ def get_api_token(sandbox: bool, else: api_token = config['zenodo']['api_token'] if not api_token: - raise ValueError(f'No API token found in {zenodo_ini_filename}. Please verify the correctness of the file ' - f'{zenodo_ini_filename}. The api_token entry must be in the section [zenodo] or ' - f'[zenodo:sandbox].') + warnings.warn(f'No API token found in {zenodo_ini_filename}. ' + f'Accessing Zenodo without API token may work, but if not it is because ' + f'of the missing api_token.') + api_token = '' + # if not api_token: + # raise ValueError(f'No API token found in {zenodo_ini_filename}. Please verify the correctness of the file ' + # f'{zenodo_ini_filename}. The api_token entry must be in the section [zenodo] or ' + # f'[zenodo:sandbox].') return api_token diff --git a/h5rdmtoolbox/database/zenodo/deposit.py b/h5rdmtoolbox/database/zenodo/deposit.py index 0d8543dc..f8deec97 100644 --- a/h5rdmtoolbox/database/zenodo/deposit.py +++ b/h5rdmtoolbox/database/zenodo/deposit.py @@ -15,7 +15,9 @@ class AbstractZenodoRecord(abc.ABC): """An abstract Zenodo record.""" base_url = None - def __init__(self, metadata: Metadata, deposit_id: Union[int, None] = None): + def __init__(self, + deposit_id: Union[int, None] = None, + metadata: Metadata = None): if self.base_url is None: raise ValueError('The base_url must be set.') self.deposit_id = deposit_id @@ -66,34 +68,40 @@ def get_filenames(self): class ZenodoRecordInterface(AbstractZenodoRecord): + """A Zenodo record interface.""" def __init__(self, - metadata: Metadata, deposit_id: Union[int, None] = None, - file_or_filenames: Union[List[Union[str, pathlib.Path]], None] = None): - self.deposit_id = int(deposit_id) - self.metadata = metadata - self.filenames = [] + metadata: Metadata = None): + self.deposit_id = deposit_id + self._metadata = metadata - if file_or_filenames is not None: - if isinstance(file_or_filenames, (str, pathlib.Path)): - self.add_file(file_or_filenames) - elif isinstance(file_or_filenames, (tuple, list)): - self.add_file(*file_or_filenames) - else: - raise TypeError( - 'file_or_filenames must be a string, pathlib.Path, or a list of strings or pathlib.Path. ' - f'Got {type(file_or_filenames)} instead.') + @property + def metadata(self): + if self._metadata is None: + raise ValueError('The metadata must be set.') + return self._metadata + + @metadata.setter + def metadata(self, value: Metadata): + if not isinstance(value, Metadata): + raise TypeError('The metadata must be a Metadata object.') + self._metadata = value def _get(self, raise_for_status: bool = False): """Get the deposit from Zenodo.""" if self.deposit_id is None: raise ValueError('The deposit_id must be set.') base_url = self.base_url + f'/{self.deposit_id}' - r = requests.get( - base_url, - params={'access_token': self.api_token} - ) + if not self.api_token: + r = requests.get( + base_url, + ) + else: + r = requests.get( + base_url, + params={'access_token': self.api_token} + ) if raise_for_status: r.raise_for_status() return r @@ -128,36 +136,31 @@ def create(self): self.deposit_id = r.json()['id'] else: assert self.deposit_id == r.json()['id'] - self._push_files() return self.deposit_id - def _push_files(self): - """Push the files to the deposit.""" - - bucket_url = self._get().json()["links"]["bucket"] - for filename in self.filenames: - logger.debug(f'adding file "{filename}" to deposit "{self.deposit_id}"') - with open(filename, "rb") as fp: - r = requests.put( - "%s/%s" % (bucket_url, filename.name), - data=fp, - params={"access_token": self.api_token}, - ) - r.raise_for_status() - def delete(self): """Delete the deposit on Zenodo.""" logger.debug(f'getting deposit "{self.deposit_id}"') - r = requests.get( - f'https://sandbox.zenodo.org/api/deposit/depositions/{self.deposit_id}', - params={'access_token': self.api_token} - ) + if not self.api_token: + r = requests.get( + f'https://sandbox.zenodo.org/api/deposit/depositions/{self.deposit_id}', + ) + else: + r = requests.get( + f'https://sandbox.zenodo.org/api/deposit/depositions/{self.deposit_id}', + params={'access_token': self.api_token} + ) r.raise_for_status() logger.debug('deleting deposit {self.deposit_id}') - r = requests.delete( - 'https://sandbox.zenodo.org/api/deposit/depositions/{}'.format(r.json()['id']), - params={'access_token': self.api_token} - ) + if not self.api_token: + r = requests.delete( + 'https://sandbox.zenodo.org/api/deposit/depositions/{}'.format(r.json()['id']), + ) + else: + r = requests.delete( + 'https://sandbox.zenodo.org/api/deposit/depositions/{}'.format(r.json()['id']), + params={'access_token': self.api_token} + ) r.raise_for_status() def update(self): @@ -170,8 +173,16 @@ def add_file(self, filename): """Add a file to the deposit.""" filename = pathlib.Path(filename) if not filename.exists(): - raise FileNotFoundError(f'{filename} does not exist.') - self.filenames.append(filename) + raise FileNotFoundError(f'File "{filename}" does not exist.') + bucket_url = self._get().json()["links"]["bucket"] + logger.debug(f'adding file "{filename}" to deposit "{self.deposit_id}"') + with open(filename, "rb") as fp: + r = requests.put( + "%s/%s" % (bucket_url, filename.name), + data=fp, + params={"access_token": self.api_token}, + ) + r.raise_for_status() def get_filenames(self): """Get a list of all filenames.""" @@ -191,8 +202,11 @@ def download_files(self, target_folder: Union[str, pathlib.Path] = None): target_folder = pathlib.Path(target_folder) fname = f["filename"] target_filename = target_folder / fname - bucket_dict = requests.get(f['links']['self'], - params={'access_token': self.api_token}).json() + if not self.api_token: + bucket_dict = requests.get(f['links']['self']).json() + else: + bucket_dict = requests.get(f['links']['self'], + params={'access_token': self.api_token}).json() logger.debug(f'downloading file "{fname}" to "{target_filename}"') download_files.append(target_filename) with open(target_filename, 'wb') as file: @@ -212,8 +226,11 @@ def download_file(self, name, target_folder: Union[str, pathlib.Path] = None): if f['filename'] == name: fname = f["filename"] target_filename = target_folder / fname - bucket_dict = requests.get(f['links']['self'], - params={'access_token': self.api_token}).json() + if not self.api_token: + bucket_dict = requests.get(f['links']['self']).json() + else: + bucket_dict = requests.get(f['links']['self'], + params={'access_token': self.api_token}).json() logger.debug(f'downloading file "{fname}" to "{target_filename}"') with open(target_filename, 'wb') as file: file.write(requests.get(bucket_dict['links']['self']).content) @@ -221,11 +238,13 @@ def download_file(self, name, target_folder: Union[str, pathlib.Path] = None): class ZenodoRecord(ZenodoRecordInterface): + """A Zenodo record in the production environment.""" base_url = 'https://zenodo.org/api/deposit/depositions' @property - def api_token(self): - return self.api_token(sandbox=False) + def api_token(self) -> str: + """Get the API token for the production environment.""" + return get_api_token(sandbox=False) class ZenodoSandboxRecord(ZenodoRecordInterface): diff --git a/tests/conventions/standard_attributes/standard_names/test_transformations_and_affixes.py b/tests/conventions/standard_attributes/standard_names/test_transformations_and_affixes.py index 5960b371..6c3b042d 100644 --- a/tests/conventions/standard_attributes/standard_names/test_transformations_and_affixes.py +++ b/tests/conventions/standard_attributes/standard_names/test_transformations_and_affixes.py @@ -43,6 +43,9 @@ def setUp(self) -> None: def test_adding_transformation(self): snt = h5tbx.conventions.standard_names.StandardNameTable.from_zenodo(doi=8276716) + from h5rdmtoolbox.database.zenodo.deposit import ZenodoRecord + z = ZenodoRecord(deposit_id=8276716) + self.assertTrue(z.exists()) # check if the problem really exists: with self.assertRaises(errors.StandardNameError): diff --git a/tests/conventions/test_conventions.py b/tests/conventions/test_conventions.py index 3fdf8365..e7b43f15 100644 --- a/tests/conventions/test_conventions.py +++ b/tests/conventions/test_conventions.py @@ -55,13 +55,13 @@ def test_upload_convention(self): publication_date=datetime.now(), ) zsr = ZenodoSandboxRecord(deposit_id=ZENODO_TUTORIAL_CONVENTION_DEPOSIT_ID, - metadata=meta, - file_or_filenames=cv_yaml_filename) + metadata=meta) if not zsr.exists(): zsr.deposit_id = None zsr.create() else: zsr.update() + zsr.add_file(cv_yaml_filename) # download file from zenodo deposit: self.assertEqual(1, len(zsr.get_filenames())) diff --git a/tests/database/test_zenodo.py b/tests/database/test_zenodo.py index a12ed25a..51e511ba 100644 --- a/tests/database/test_zenodo.py +++ b/tests/database/test_zenodo.py @@ -70,8 +70,8 @@ def test_create_new_deposit(self): self.assertFalse(zsr.exists()) with open('testfile.txt', 'w') as f: f.write('This is a test file.') - zsr.add_file('testfile.txt') zsr.create() + zsr.add_file('testfile.txt') zsr.create() # call it again. does it crash? pathlib.Path('testfile.txt').unlink() self.assertTrue(zsr.exists())