Skip to content

Commit

Permalink
(Archiving) Fix behaviour for non-positive limits (#2737) (patch)
Browse files Browse the repository at this point in the history
### Fixed

- A provided limit of a non-positive integer now results in an exit.
  • Loading branch information
islean authored Dec 6, 2023
1 parent 9a6ec23 commit d1d1f6a
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 27 deletions.
7 changes: 5 additions & 2 deletions cg/meta/archive/archive.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,15 @@ def archive_files_to_location(
def archive_spring_files_and_add_archives_to_housekeeper(
self, spring_file_count_limit: int | None
) -> None:
"""Archives all non archived spring files. If a limit is provided, the amount of files archived are limited
"""Archives all non-archived spring files. If a limit is provided, the amount of files archived is limited
to that amount."""
if isinstance(spring_file_count_limit, int) and spring_file_count_limit <= 0:
LOG.warning("Please do not provide a non-positive integer as limit - exiting.")
return
for archive_location in ArchiveLocations:
files_to_archive: list[File] = self.housekeeper_api.get_non_archived_spring_files(
tags=[archive_location],
limit=spring_file_count_limit if spring_file_count_limit else None,
limit=spring_file_count_limit,
)
if files_to_archive:
files_and_samples_for_location = self.add_samples_to_files(files_to_archive)
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ urllib3 = "*"

# Apps
genologics = "*"
housekeeper = "*"
housekeeper = ">=4.10.14"


[tool.poetry.dev-dependencies]
Expand Down
9 changes: 9 additions & 0 deletions tests/meta/archive/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,12 +287,21 @@ def spring_archive_api(
archive_store: Store,
ddn_dataflow_config: DataFlowConfig,
father_sample_id: str,
sample_id: str,
helpers,
) -> SpringArchiveAPI:
"""Returns an ArchiveAPI with a populated housekeeper store and a DDNDataFlowClient.
Also adds /home/ as a prefix for each SPRING file for the DDNDataFlowClient to accept them."""
populated_housekeeper_api.add_tags_if_non_existent(
tag_names=[ArchiveLocations.KAROLINSKA_BUCKET]
)
for spring_file in populated_housekeeper_api.files(tags=[SequencingFileTag.SPRING]):
spring_file.path = f"/home/{spring_file.path}"
if spring_file.version.bundle.name == sample_id:
spring_file.tags.append(
populated_housekeeper_api.get_tag(name=ArchiveLocations.KAROLINSKA_BUCKET)
)

populated_housekeeper_api.commit()
return SpringArchiveAPI(
housekeeper_api=populated_housekeeper_api,
Expand Down
50 changes: 26 additions & 24 deletions tests/meta/archive/test_archive_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import pytest
from housekeeper.store.models import File

from cg.apps.housekeeper.hk import HousekeeperAPI
from cg.constants.archiving import ArchiveLocations
from cg.constants.constants import APIMethods
from cg.constants.housekeeper_tags import SequencingFileTag
Expand Down Expand Up @@ -148,6 +147,7 @@ def test_call_corresponding_archiving_method(spring_archive_api: SpringArchiveAP
mock_request_submitter.assert_called_once_with(files_and_samples=[file_and_sample])


@pytest.mark.parametrize("limit", [None, -1, 0, 1])
def test_archive_all_non_archived_spring_files(
spring_archive_api: SpringArchiveAPI,
caplog,
Expand All @@ -156,6 +156,7 @@ def test_archive_all_non_archived_spring_files(
header_with_test_auth_token,
test_auth_token: AuthToken,
sample_id: str,
limit: int | None,
):
"""Test archiving all non-archived SPRING files for Miria customers."""
# GIVEN a populated status_db database with two customers, one DDN and one non-DDN,
Expand All @@ -166,36 +167,37 @@ def test_archive_all_non_archived_spring_files(
AuthToken,
"model_validate",
return_value=test_auth_token,
), mock.patch.object(
HousekeeperAPI,
"get_non_archived_spring_files",
return_value=[spring_archive_api.housekeeper_api.get_files(bundle=sample_id).first()],
), mock.patch.object(
APIRequest,
"api_request_from_content",
return_value=ok_miria_response,
) as mock_request_submitter:
spring_archive_api.archive_spring_files_and_add_archives_to_housekeeper(200)
spring_archive_api.archive_spring_files_and_add_archives_to_housekeeper(
spring_file_count_limit=limit
)

# THEN the DDN archiving function should have been called with the correct destination and source.
mock_request_submitter.assert_called_with(
api_method=APIMethods.POST,
url="some/api/files/archive",
headers=header_with_test_auth_token,
json=archive_request_json,
verify=False,
)
# THEN the DDN archiving function should have been called with the correct destination and source if limit > 0
if limit not in [0, -1]:
mock_request_submitter.assert_called_with(
api_method=APIMethods.POST,
url="some/api/files/archive",
headers=header_with_test_auth_token,
json=archive_request_json,
verify=False,
)

# THEN all spring files for Karolinska should have an entry in the Archive table in Housekeeper while no other
# files should have an entry
files: list[File] = spring_archive_api.housekeeper_api.files()
for file in files:
if SequencingFileTag.SPRING in [tag.name for tag in file.tags]:
sample: Sample = spring_archive_api.status_db.get_sample_by_internal_id(
file.version.bundle.name
)
if sample and sample.archive_location == ArchiveLocations.KAROLINSKA_BUCKET:
assert file.archive
# THEN all spring files for Karolinska should have an entry in the Archive table in Housekeeper while no other
# files should have an entry
files: list[File] = spring_archive_api.housekeeper_api.files()
for file in files:
if SequencingFileTag.SPRING in [tag.name for tag in file.tags]:
sample: Sample = spring_archive_api.status_db.get_sample_by_internal_id(
file.version.bundle.name
)
if sample and sample.archive_location == ArchiveLocations.KAROLINSKA_BUCKET:
assert file.archive
else:
mock_request_submitter.assert_not_called()


@pytest.mark.parametrize(
Expand Down

0 comments on commit d1d1f6a

Please sign in to comment.