Skip to content

Commit

Permalink
[COST-5028] guard against report_status not existing (#5099)
Browse files Browse the repository at this point in the history
  • Loading branch information
maskarb authored May 10, 2024
1 parent 5e23153 commit 764a911
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ def get_ocp_provider_uuids_tuple(self):
ocp_provider_uuids.append(ocp_provider_uuid)
return tuple(ocp_provider_uuids)

def process(self, parquet_base_filename, daily_data_frames, manifest_id=None):
def process(self, parquet_base_filename, daily_data_frames):
"""Filter data and convert to parquet."""
if not (ocp_provider_uuids := self.get_ocp_provider_uuids_tuple()):
return
Expand All @@ -243,7 +243,10 @@ def process(self, parquet_base_filename, daily_data_frames, manifest_id=None):
)
return

self.report_status.update_status(CombinedChoices.OCP_CLOUD_PROCESSING)
if self.report_status:
# internal masu endpoints may result in this being None, so guard this in case there is no status to update
self.report_status.update_status(CombinedChoices.OCP_CLOUD_PROCESSING)

# # Get OpenShift topology data
with OCPReportDBAccessor(self.schema_name) as accessor:
if self.provider_type == Provider.PROVIDER_GCP:
Expand Down
20 changes: 12 additions & 8 deletions koku/masu/processor/parquet/parquet_report_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,12 @@ def __init__(
ingress_reports_uuid=None,
):
"""initialize report processor."""
if context is None:
context = {}
self._schema_name = schema_name
self._provider_uuid = provider_uuid
self._report_file = Path(report_path)
self.provider_type = provider_type
self._manifest_id = manifest_id
self._context = context
self._context = context or {}
self.start_date = self._context.get("start_date")
self.invoice_month_date = None
if invoice_month := self._context.get("invoice_month"):
Expand All @@ -98,10 +96,6 @@ def __init__(

self.split_files = [Path(file) for file in self._context.get("split_files") or []]
self.ocp_files_to_process: dict[str, dict[str, str]] = self._context.get("ocp_files_to_process")
if self.manifest_id:
self.report_status = CostUsageReportStatus.objects.get(
report_name=Path(self._report_file).name, manifest_id=self.manifest_id
)

@property
def schema_name(self):
Expand Down Expand Up @@ -149,6 +143,13 @@ def manifest_id(self):
"""The manifest id."""
return self._manifest_id

@cached_property
def report_status(self):
if self.manifest_id:
return CostUsageReportStatus.objects.get(
report_name=Path(self._report_file).name, manifest_id=self.manifest_id
)

@property
def report_file(self):
"""The complete CSV file path."""
Expand Down Expand Up @@ -582,7 +583,10 @@ def convert_csv_to_parquet(self, csv_filename: os.PathLike): # noqa: C901
),
exc_info=err,
)
self.report_status.update_status(CombinedChoices.FAILED)
if self.report_status:
# internal masu endpoints may result in this being None,
# so guard this in case there is no status to update
self.report_status.update_status(CombinedChoices.FAILED)
return parquet_base_filename, daily_data_frames, False

return parquet_base_filename, daily_data_frames, True
Expand Down
2 changes: 1 addition & 1 deletion koku/masu/processor/report_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def process(self):
try:
parquet_base_filename, daily_data_frames = self._processor.process()
if self.ocp_on_cloud_processor:
self.ocp_on_cloud_processor.process(parquet_base_filename, daily_data_frames, self.manifest_id)
self.ocp_on_cloud_processor.process(parquet_base_filename, daily_data_frames)
return daily_data_frames != []
except ReportsAlreadyProcessed:
report_status.update_status(CombinedChoices.DONE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from masu.util.aws.common import match_openshift_resources_and_labels
from masu.util.gcp.common import match_openshift_resources_and_labels as gcp_match_openshift_resources_and_labels
from reporting.provider.all.models import EnabledTagKeys
from reporting_common.models import CostUsageReportStatus


class TestOCPCloudParquetReportProcessor(MasuTestCase):
Expand Down Expand Up @@ -443,3 +444,20 @@ def test_get_matched_tags_trino(self, mock_has_enabled, mock_matching_enabled, m
):
self.report_processor.get_matched_tags([])
mock_get_tags.assert_not_called()

def test_instantiating_processor_without_manifest_id(self):
"""Assert that report_status exists and is None."""
report_processor = OCPCloudParquetReportProcessor(
schema_name=self.schema,
report_path=self.report_path,
provider_uuid=self.gcp_provider_uuid,
provider_type=Provider.PROVIDER_GCP_LOCAL,
manifest_id=0,
context={"request_id": self.request_id, "start_date": self.start_date, "create_table": True},
)
self.assertIsNone(report_processor.report_status)

def test_instantiating_processor_with_manifest_id(self):
"""Assert that report_status exists and is not None."""
self.assertIsNotNone(self.report_processor.report_status)
self.assertIsInstance(self.report_processor.report_status, CostUsageReportStatus)

0 comments on commit 764a911

Please sign in to comment.