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-5024] Avoid custom JSON parsing for GCP credits field #5103

Merged
merged 6 commits into from
May 16, 2024

Conversation

samdoran
Copy link
Contributor

@samdoran samdoran commented May 13, 2024

Jira Ticket

COST-5024

Description

Do not modify a valid JSON string in the GCP credits field. Fall back to custom conversion if the string is invalid JSON. Finally, warn if there is still an error.

Testing

tox -e py39 -- masu.test.util.gcp.test_gcp_post_processor

Release Notes

  • proposed release note
* [COST-5024](https://issues.redhat.com/browse/COST-5024) Avoid use of custom JSON parsing for GCP credits if possible (#5103)

@samdoran samdoran added the gcp-smoke-tests pr_check will build the image and run gcp + ocp on gcp smoke tests label May 13, 2024
Copy link

codecov bot commented May 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.2%. Comparing base (cbe5e5c) to head (d800d2e).

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #5103   +/-   ##
=====================================
  Coverage   94.2%   94.2%           
=====================================
  Files        378     378           
  Lines      31581   31583    +2     
  Branches    3754    3754           
=====================================
+ Hits       29735   29739    +4     
+ Misses      1176    1175    -1     
+ Partials     670     669    -1     

@samdoran samdoran force-pushed the COST-5024/gcp-credits-converter branch from 97c262e to 2e0206b Compare May 14, 2024 17:19
@samdoran samdoran marked this pull request as ready for review May 14, 2024 17:19
@samdoran samdoran requested review from a team as code owners May 14, 2024 17:19
koku/masu/util/gcp/gcp_post_processor.py Outdated Show resolved Hide resolved
koku/masu/util/gcp/gcp_post_processor.py Outdated Show resolved Hide resolved
Comment on lines 140 to 143
rollup_frame["daily_credits"] = 0.0
for i, credit_dict in enumerate(rollup_frame["credits"]):
# FIXME: Remove chained assignment
rollup_frame["daily_credits"][i] = credit_dict.get("amount", 0.0)
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding a FIXME, we could just fix this:

Suggested change
rollup_frame["daily_credits"] = 0.0
for i, credit_dict in enumerate(rollup_frame["credits"]):
# FIXME: Remove chained assignment
rollup_frame["daily_credits"][i] = credit_dict.get("amount", 0.0)
rollup_frame["daily_credits"] = rollup_frame["credits"].apply(lambda x: x.get("amount", 0.0))

@maskarb maskarb enabled auto-merge (squash) May 16, 2024 13:56
@maskarb maskarb merged commit febeb57 into main May 16, 2024
11 checks passed
@maskarb maskarb deleted the COST-5024/gcp-credits-converter branch May 16, 2024 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gcp-smoke-tests pr_check will build the image and run gcp + ocp on gcp smoke tests smokes-required
Projects
None yet
3 participants