Skip to content

Commit

Permalink
Fix distributing pricing service cost when there is lack of some dail…
Browse files Browse the repository at this point in the history
…y usages

When pricing service has multiple services (for distributing cost) and
not every DailyUsage is present (see #648 - ignoring zero-value usage)
costs were not distributed properly. It was caused by storying usage
values for single service environment on lists - these lists have
different size and when accessing by index (and using `zip` to
join this list with total usage and percentage for usage type)
usage value was combined with wrong total usage and percentage.
  • Loading branch information
mkurek committed Dec 4, 2018
1 parent 1e0b14c commit 392b83e
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 9 deletions.
21 changes: 12 additions & 9 deletions src/ralph_scrooge/plugins/cost/pricing_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,11 @@ def _distribute_costs(
...
}
"""
usages = defaultdict(list)
total_usages = []
percentage = []
usages = defaultdict(dict)
total_usages = {}
percentage = {}
result = defaultdict(list)
self.pricing_service = pricing_service

for service_usage_type in service_usage_types:
service_excluded = excluded_services.union(
service_usage_type.usage_type.excluded_services.all()
Expand All @@ -255,18 +254,22 @@ def _distribute_costs(
'daily_pricing_object__pricing_object',
'service_environment',
).annotate(usage=Sum('value'))
usage_type_id = service_usage_type.usage_type_id
for pricing_object, se, usage in usages_per_po:
usages[(pricing_object, se)].append(usage)
usages[(pricing_object, se)][usage_type_id] = usage

total_usages.append(self._get_total_usage(
total_usages[usage_type_id] = self._get_total_usage(
usage_type=service_usage_type.usage_type,
date=date,
excluded_services=service_excluded,
))
percentage.append(service_usage_type.percent)
)
percentage[usage_type_id] = service_usage_type.percent
# create hierarchy basing on usages
for (po, se), po_usages in usages.items():
po_usages_info = zip(po_usages, total_usages, percentage)
po_usages_info = [
(value, total_usages[ut_id], percentage[ut_id])
for ut_id, value in po_usages.items()
]
result[se].extend(
self._add_hierarchy_costs(po, po_usages_info, costs_hierarchy)
)
Expand Down
66 changes: 66 additions & 0 deletions src/ralph_scrooge/tests/plugins/cost/test_pricing_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,72 @@ def test_distribute_costs_with_excluded_services(self):
]),
)

def test_distribute_costs_with_incomplete_multiple_usages(self):
self._init_one()

# delete some daily usages
models.DailyUsage.objects.get(
daily_pricing_object=self.dpo1,
date=self.today,
type=self.service_usage_types[0]
).delete()
models.DailyUsage.objects.get(
daily_pricing_object=self.dpo2,
date=self.today,
type=self.service_usage_types[1]
).delete()

# usages right now:
# | percent | dpo1 | dpo2 | dpo3 | total
# SU1 | 70% | - | 10 | 10 | 20
# SU2 | 30% | 20 | - | 20 | 40

distributed_costs = PricingServicePlugin._distribute_costs(
date=self.today,
pricing_service=self.ps1,
costs_hierarchy={
self.ps1.id: (
D(1000),
{
self.service_usage_types[0].id: (D(300), {}),
self.service_usage_types[1].id: (D(700), {}),
}
)
},
service_usage_types=self.ps1.serviceusagetypes_set.all(),
excluded_services=set()
)

expected = {
self.dpo1.service_environment_id: [
{
'pricing_object_id': self.dpo1.pricing_object.id,
# 70% * 1000 * 0 / 20 + 30% * 1000 * 20 / 40
'cost': D('150.00'),
'type_id': self.ps1.id,
'value': 20,
}
],
self.dpo2.service_environment_id: [
{
'pricing_object_id': self.dpo2.pricing_object.id,
# 70% * 1000 * 10 / 20 + 30% * 1000 * 0 / 40
'cost': D('350.00'),
'value': 10,
'type_id': self.ps1.id,
}
],
self.dpo3.service_environment_id: [
{
'pricing_object_id': self.dpo3.pricing_object.id,
# 70% * 1000 * 10 / 20 + 30% * 1000 * 20 / 40
'cost': D('500.00'),
'type_id': self.ps1.id,
}
],
}
self.assertEqual(dict(distributed_costs), expected)

def test_pricing_dependent_services(self):
"""
Call costs for PS1, which dependent on PS2, which dependent of PS3.
Expand Down

0 comments on commit 392b83e

Please sign in to comment.