Skip to content

Commit

Permalink
Address feedback by using report_zip directly as IO[byte] stream and …
Browse files Browse the repository at this point in the history 
…move bucket lifecycle configuration to terraform.
  • Loading branch information
@dbernstein
dbernstein committed Dec 16, 2024
1 parent cdd8987 commit fe5dc35
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 141 deletions.
29 changes: 10 additions & 19 deletions src/palace/manager/celery/tasks/generate_inventory_and_hold_reports.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,33 +148,24 @@ def run(self) -> None:
arcname=f"palace-inventory-report-for-library-{file_name_modifier}.csv",
)

reports_path = "inventory_and_holds"
expiration_in_days = 30
self.s3_service.update_bucket_expiration_rule(
prefix=f"{reports_path}/", expiration_in_days=expiration_in_days
uid = uuid.uuid4()
key = (
f"inventory_and_holds/{library.short_name}/"
f"inventory-and-holds-for-library-{file_name_modifier}-{uid}.zip"
)
self.s3_service.store_stream(
key,
report_zip,
content_type="application/zip",
)

with zip_path.open(
"rb",
) as binary_stream:
uid = uuid.uuid4()
key = (
f"{reports_path}/{library.short_name}/"
f"inventory-and-holds-for-library-{file_name_modifier}-{uid}.zip"
)
self.s3_service.store_stream(
key,
binary_stream,
content_type="application/zip",
)

s3_file_link = self.s3_service.generate_url(key)
self.send_email(
subject=f"Inventory and Holds Reports {current_time}",
receivers=[self.email_address],
text=(
f"Download Report here -> {s3_file_link} \n\n"
f"This report will be available for download for {expiration_in_days} days."
f"This report will be available for download for 30 days."
),
)

Expand Down
57 changes: 0 additions & 57 deletions src/palace/manager/service/storage/s3.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,60 +244,3 @@ def multipart_abort(self, key: str, upload_id: str) -> None:
Key=key,
UploadId=upload_id,
)

def update_bucket_expiration_rule(
self, prefix: str, expiration_in_days: int = 30
) -> None:
"""
Update the expiration lifecycle policy rule if it exists and has changed. If
it does not already exist, add one.
"""
rule_name = f"expiration_on_{prefix}"
bucket = self.bucket
configuration = self.client.get_bucket_lifecycle_configuration(Bucket=bucket)

rules_list = configuration["Rules"]
for rule in rules_list:
if "ID" in rule and rule["ID"] == rule_name:
if (
rule["Prefix"] == prefix
and rule["Expiration"]["Days"] == expiration_in_days
):
self.log.info(
f"Expiration lifecycle rule {rule_name} has not changed for {bucket}: "
f"current value = {rule}. Ignoring."
)
return
else:
# remove the old version of the rule from the list
rules_list.remove(rule)
break

rules_list.append(
{
"Expiration": {
"Days": expiration_in_days,
},
"ID": rule_name,
"Prefix": prefix,
"Filter": {
"Prefix": prefix,
},
"Status": "Enabled",
}
)

try:
policy_status = self.client.put_bucket_lifecycle_configuration(
Bucket=bucket, LifecycleConfiguration={"Rules": rules_list} # type: ignore[typeddict-item]
)

self.log.info(
f"Updated bucket lifecycle configuration for {bucket}: "
f"configuration={configuration}, "
f"status = {policy_status}"
)
except ClientError as e:
self.log.error(
f"Unable to apply bucket lifecycle configuration for {self.bucket}. \nReason:{e}"
)
2 changes: 0 additions & 2 deletions tests/manager/celery/tasks/test_generate_inventory_and_hold_reports.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,6 @@ def store_stream_mock(

job.run()

mock_s3.update_bucket_expiration_rule.assert_called_once()
mock_s3.generate_url.assert_called_once()
send_email_mock.assert_called_once()
kwargs = send_email_mock.call_args.kwargs
Expand Down Expand Up @@ -319,7 +318,6 @@ def test_generate_inventory_and_hold_reports_task(
services_fixture.email_fixture.mock_emailer.send.assert_called_once()

mock_s3_service.store_stream.assert_called_once()
mock_s3_service.update_bucket_expiration_rule.assert_called_once()
mock_s3_service.generate_url.assert_called_once()

assert (
Expand Down
63 changes: 0 additions & 63 deletions tests/manager/service/storage/test_s3.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,69 +240,6 @@ def _configuration(self, prefix, expiration):
]
}

def test_update_bucket_expiration_rule_not_previously_set(
self, s3_service_fixture: S3ServiceFixture
):
service = s3_service_fixture.service()
prefix = "prefix/"
expiration_in_days = 10

s3_service_fixture.mock_s3_client.get_bucket_lifecycle_configuration.return_value = {
"Rules": []
}
service.update_bucket_expiration_rule(
prefix=prefix, expiration_in_days=expiration_in_days
)
s3_service_fixture.mock_s3_client.get_bucket_lifecycle_configuration.assert_called_once()
put_config_method = (
s3_service_fixture.mock_s3_client.put_bucket_lifecycle_configuration
)
put_config_method.assert_called_once_with(
Bucket="bucket",
LifecycleConfiguration=self._configuration(prefix, expiration_in_days),
)

def test_update_bucket_expiration_rule_previously_set_unchanged(
self, s3_service_fixture: S3ServiceFixture
):
service = s3_service_fixture.service()
prefix = "prefix/"
expiration_in_days = 10

get_bucket_lifecycle_config = (
s3_service_fixture.mock_s3_client.get_bucket_lifecycle_configuration
)
get_bucket_lifecycle_config.return_value = self._configuration(
prefix, expiration_in_days
)
service.update_bucket_expiration_rule(
prefix=prefix, expiration_in_days=expiration_in_days
)
s3_service_fixture.mock_s3_client.get_bucket_lifecycle_configuration.assert_called_once()
s3_service_fixture.mock_s3_client.put_bucket_lifecycle_configuration.assert_not_called()

def test_update_bucket_expiration_rule_previously_set_changed(
self, s3_service_fixture: S3ServiceFixture
):
service = s3_service_fixture.service()
prefix = "prefix/"
expiration_in_days = 10
get_lifecycle = (
s3_service_fixture.mock_s3_client.get_bucket_lifecycle_configuration
)
get_lifecycle.return_value = self._configuration(prefix, 12)
service.update_bucket_expiration_rule(
prefix=prefix, expiration_in_days=expiration_in_days
)
s3_service_fixture.mock_s3_client.get_bucket_lifecycle_configuration.assert_called_once()
put_config_method = (
s3_service_fixture.mock_s3_client.put_bucket_lifecycle_configuration
)
put_config_method.assert_called_once_with(
Bucket="bucket",
LifecycleConfiguration=self._configuration(prefix, expiration_in_days),
)


@pytest.mark.minio
class TestS3ServiceIntegration:
Expand Down

0 comments on commit fe5dc35

Please sign in to comment.