Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[COST-5028] guard against report_status not existing #5099

Merged
merged 4 commits into from
May 10, 2024

Conversation

maskarb
Copy link
Member

@maskarb maskarb commented May 10, 2024

Jira Ticket

COST-5028

Description

This change will prevent task failures when report_status does not exist for OCPCloudParquetReportProcessor.

Testing

starting on main:

  1. load test data (make load-test-customer-data)
  2. trigger ocp-on-cloud resummary:
http://localhost:5042/api/cost-management/v1/report/process/openshift_on_cloud/?schema=org1234567&provider_uuid={any cloud source uuid}&start_date=2024-04-01
  1. in worker logs, see task failure:
koku-worker-1  | [2024-05-10 17:18:40,119] ERROR 5b2f96c5-43f6-451e-8a02-29902e6bfc15 1160 Task failed: 'OCPCloudParquetReportProcessor' object has no attribute 'report_status'
  1. checkout branch and ensure worker reloads
  2. re-hit this endpoint:
http://localhost:5042/api/cost-management/v1/report/process/openshift_on_cloud/?schema=org1234567&provider_uuid={any cloud source uuid}&start_date=2024-04-01
  1. see the task complete successfully.

Release Notes

  • proposed release note
* [COST-5028](https://issues.redhat.com/browse/COST-5028) guard against non-existent report_status

@maskarb maskarb requested review from a team as code owners May 10, 2024 17:10
@maskarb maskarb changed the title guard against report_status not existing [COST-5028] guard against report_status not existing May 10, 2024
Copy link

codecov bot commented May 10, 2024

Codecov Report

Attention: Patch coverage is 81.81818% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 94.1%. Comparing base (5e23153) to head (7cf13a9).

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #5099     +/-   ##
=======================================
- Coverage   94.1%   94.1%   -0.0%     
=======================================
  Files        378     378             
  Lines      31577   31579      +2     
  Branches    3753    3754      +1     
=======================================
- Hits       29728   29725      -3     
- Misses      1177    1180      +3     
- Partials     672     674      +2     

@maskarb maskarb added the smoke-tests pr_check will build the image and run minimal required smokes label May 10, 2024
djnakabaale
djnakabaale previously approved these changes May 10, 2024
Copy link
Contributor

@djnakabaale djnakabaale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍🏾

return CostUsageReportStatus.objects.get(
report_name=Path(self._report_file).name, manifest_id=self.manifest_id
)
return None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return None is implicit, so it could be omitted.

@maskarb maskarb enabled auto-merge (squash) May 10, 2024 20:15
@maskarb maskarb merged commit 764a911 into main May 10, 2024
10 of 11 checks passed
@maskarb maskarb deleted the report-status-fixish branch May 10, 2024 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
smoke-tests pr_check will build the image and run minimal required smokes smokes-required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants