From 08497f0d523ab8590f76ce274a001f3e3cdb84a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Tue, 29 Sep 2020 14:23:47 -0300 Subject: [PATCH] Increase cost fields to 28 digits precision When working with some type of resources, and for some specific billing requirements, we need to set costs that will use up to more than 8 digits on the right side of the comma. By default, the Python object Decimal support 28 digits. Therefore, it makes sense for us to change the MySQL database schema of CloudKitty to use 28 digits as well on the right side. This will avoid confusion for people when using this feature. One can argue that using the `factor` option can also be a solution for that, but as I mentioned, for people used to Python, that can cause confusions because the MySQL DB is using a different precision than the one supported in Python for the data type we use to represent the `cost` field. Change-Id: Ifbf5b2515c7eaf470b48f2695d1e45eeab708a72 --- ...15c7_increase_precision_for_cost_fields.py | 45 ++++++++++ .../rating/hash/db/sqlalchemy/models.py | 4 +- .../rating/hash/gabbits/hash-location.yaml | 10 +-- .../tests/gabbi/rating/hash/gabbits/hash.yaml | 14 ++-- cloudkitty/tests/test_hashmap.py | 84 ++++++++++++------- doc/source/user/rating/hashmap.rst | 6 ++ 6 files changed, 120 insertions(+), 43 deletions(-) create mode 100644 cloudkitty/rating/hash/db/sqlalchemy/alembic/versions/Ifbf5b2515c7_increase_precision_for_cost_fields.py diff --git a/cloudkitty/rating/hash/db/sqlalchemy/alembic/versions/Ifbf5b2515c7_increase_precision_for_cost_fields.py b/cloudkitty/rating/hash/db/sqlalchemy/alembic/versions/Ifbf5b2515c7_increase_precision_for_cost_fields.py new file mode 100644 index 00000000..c5b99d41 --- /dev/null +++ b/cloudkitty/rating/hash/db/sqlalchemy/alembic/versions/Ifbf5b2515c7_increase_precision_for_cost_fields.py @@ -0,0 +1,45 @@ +# Copyright 2018 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +"""Increase cost fields to 30 digits + +Revision ID: Ifbf5b2515c7 +Revises: 644faa4491fd +Create Date: 2020-09-29 14:22:00.000000 + +""" + +from alembic import op +import importlib + +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = 'Ifbf5b2515c7' +down_revision = '644faa4491fd' + + +def upgrade(): + down_version_module = importlib.import_module( + "cloudkitty.rating.hash.db.sqlalchemy.alembic.versions." + "644faa4491fd_update_tenant_id_type_from_uuid_to_text") + + for table_name in ('hashmap_mappings', 'hashmap_thresholds'): + with op.batch_alter_table( + table_name, reflect_args=down_version_module.get_reflect( + table_name)) as batch_op: + + batch_op.alter_column('cost', + type_=sa.Numeric(precision=30, scale=28)) diff --git a/cloudkitty/rating/hash/db/sqlalchemy/models.py b/cloudkitty/rating/hash/db/sqlalchemy/models.py index a86028d5..e40214f4 100644 --- a/cloudkitty/rating/hash/db/sqlalchemy/models.py +++ b/cloudkitty/rating/hash/db/sqlalchemy/models.py @@ -230,7 +230,7 @@ class HashMapMapping(Base, HashMapBase): sqlalchemy.String(255), nullable=True) cost = sqlalchemy.Column( - sqlalchemy.Numeric(20, 8), + sqlalchemy.Numeric(30, 28), nullable=False) map_type = sqlalchemy.Column( sqlalchemy.Enum( @@ -309,7 +309,7 @@ class HashMapThreshold(Base, HashMapBase): sqlalchemy.Numeric(20, 8), nullable=True) cost = sqlalchemy.Column( - sqlalchemy.Numeric(20, 8), + sqlalchemy.Numeric(30, 28), nullable=False) map_type = sqlalchemy.Column( sqlalchemy.Enum( diff --git a/cloudkitty/tests/gabbi/rating/hash/gabbits/hash-location.yaml b/cloudkitty/tests/gabbi/rating/hash/gabbits/hash-location.yaml index a94545a1..a341604f 100644 --- a/cloudkitty/tests/gabbi/rating/hash/gabbits/hash-location.yaml +++ b/cloudkitty/tests/gabbi/rating/hash/gabbits/hash-location.yaml @@ -28,13 +28,13 @@ tests: data: service_id: "6c1b8a30-797f-4b7e-ad66-9879b79059fb" type: "flat" - cost: "0.10000000" + cost: "0.1000000000000000055511151231" status: 201 response_json_paths: $.mapping_id: "6c1b8a30-797f-4b7e-ad66-9879b79059fb" $.service_id: "6c1b8a30-797f-4b7e-ad66-9879b79059fb" $.type: "flat" - $.cost: "0.10000000" + $.cost: "0.1000000000000000055511151231" response_headers: location: '$SCHEME://$NETLOC/v1/rating/module_config/hashmap/mappings/6c1b8a30-797f-4b7e-ad66-9879b79059fb' @@ -60,7 +60,7 @@ tests: $.service_id: "6c1b8a30-797f-4b7e-ad66-9879b79059fb" $.level: "2.00000000" $.type: "flat" - $.cost: "0.10000000" + $.cost: "0.1000000000000000055511151231" response_headers: location: '$SCHEME://$NETLOC/v1/rating/module_config/hashmap/thresholds/6c1b8a30-797f-4b7e-ad66-9879b79059fb' @@ -103,7 +103,7 @@ tests: $.field_id: "6c1b8a30-797f-4b7e-ad66-9879b79059fb" $.value: "04774238-fcad-11e5-a90e-6391fd56aab2" $.type: "flat" - $.cost: "0.10000000" + $.cost: "0.1000000000000000055511151231" response_headers: location: '$SCHEME://$NETLOC/v1/rating/module_config/hashmap/mappings/6c1b8a30-797f-4b7e-ad66-9879b79059fb' @@ -124,7 +124,7 @@ tests: $.field_id: "6c1b8a30-797f-4b7e-ad66-9879b79059fb" $.level: "2.00000000" $.type: "flat" - $.cost: "0.10000000" + $.cost: "0.1000000000000000055511151231" response_headers: location: '$SCHEME://$NETLOC/v1/rating/module_config/hashmap/thresholds/6c1b8a30-797f-4b7e-ad66-9879b79059fb' diff --git a/cloudkitty/tests/gabbi/rating/hash/gabbits/hash.yaml b/cloudkitty/tests/gabbi/rating/hash/gabbits/hash.yaml index f54cf84a..a53265bd 100644 --- a/cloudkitty/tests/gabbi/rating/hash/gabbits/hash.yaml +++ b/cloudkitty/tests/gabbi/rating/hash/gabbits/hash.yaml @@ -48,7 +48,7 @@ tests: response_json_paths: $.service_id: $RESPONSE['$.service_id'] $.type: "flat" - $.cost: "0.10000000" + $.cost: "0.1000000000000000055511151231" - name: delete a flat service mapping url: /v1/rating/module_config/hashmap/mappings/$RESPONSE['$.mapping_id'] @@ -76,7 +76,7 @@ tests: response_json_paths: $.service_id: $RESPONSE['$.services[0].service_id'] $.type: "rate" - $.cost: "0.20000000" + $.cost: "0.2000000000000000111022302463" - name: create a flat service mapping for a tenant url: /v1/rating/module_config/hashmap/mappings @@ -93,7 +93,7 @@ tests: response_json_paths: $.service_id: $ENVIRON['hash_service_id'] $.type: "flat" - $.cost: "0.20000000" + $.cost: "0.2000000000000000111022302463" $.tenant_id: "24a7fdae-27ff-11e6-8c4f-6b725a05bf50" - name: list service mappings no tenant filtering @@ -131,7 +131,7 @@ tests: $.service_id: $ENVIRON['hash_service_id'] $.level: "2.00000000" $.type: "flat" - $.cost: "0.20000000" + $.cost: "0.2000000000000000111022302463" $.tenant_id: "24a7fdae-27ff-11e6-8c4f-6b725a05bf50" - name: list service thresholds no tenant filtering @@ -191,7 +191,7 @@ tests: response_json_paths: $.field_id: $RESPONSE['$.field_id'] $.type: "rate" - $.cost: "0.20000000" + $.cost: "0.2000000000000000111022302463" - name: delete a flat field mapping url: /v1/rating/module_config/hashmap/mappings/$RESPONSE['$.mapping_id'] @@ -222,7 +222,7 @@ tests: response_json_paths: $.field_id: $RESPONSE['$.fields[0].field_id'] $.type: "rate" - $.cost: "0.20000000" + $.cost: "0.2000000000000000111022302463" response_store_environ: hash_rate_mapping_id: $.mapping_id @@ -245,7 +245,7 @@ tests: $.mapping_id: $ENVIRON['hash_rate_mapping_id'] $.field_id: $ENVIRON['hash_field_id'] $.type: "rate" - $.cost: "0.30000000" + $.cost: "0.2999999999999999888977697537" $.value: "f17a0674-0004-11e6-a16b-cf941f4668c4" - name: delete a field diff --git a/cloudkitty/tests/test_hashmap.py b/cloudkitty/tests/test_hashmap.py index f655af82..83e3c9b6 100644 --- a/cloudkitty/tests/test_hashmap.py +++ b/cloudkitty/tests/test_hashmap.py @@ -343,7 +343,8 @@ class HashMapRatingTest(tests.TestCase): mapping = self._db_api.get_mapping(mapping_db.mapping_id) self.assertEqual('flat', mapping.map_type) self.assertEqual('m1.tiny', mapping.value) - self.assertEqual(decimal.Decimal('1.337'), mapping.cost) + self.assertEqual( + decimal.Decimal('1.3369999999999999662492200514'), mapping.cost) self.assertEqual(field_db.id, mapping.field_id) def test_list_mappings_from_services(self): @@ -524,7 +525,8 @@ class HashMapRatingTest(tests.TestCase): threshold = self._db_api.get_threshold(threshold_db.threshold_id) self.assertEqual('rate', threshold.map_type) self.assertEqual(decimal.Decimal('64'), threshold.level) - self.assertEqual(decimal.Decimal('0.1337'), threshold.cost) + self.assertEqual( + decimal.Decimal('0.1337000000000000132782673745'), threshold.cost) self.assertEqual(field_db.id, threshold.field_id) def test_list_thresholds_from_only_group(self): @@ -768,11 +770,13 @@ class HashMapRatingTest(tests.TestCase): 'mappings': { '_DEFAULT_': { 'm1.tiny': { - 'cost': decimal.Decimal('2'), + 'cost': decimal.Decimal( + '2.0000000000000000000000000000'), 'type': 'flat'}}, 'test_group': { 'm1.large': { - 'cost': decimal.Decimal('13.37'), + 'cost': decimal.Decimal( + '13.3699999999999992184029906639'), 'type': 'rate'}}}, 'thresholds': {}}, 'memory': { @@ -780,14 +784,17 @@ class HashMapRatingTest(tests.TestCase): 'thresholds': { 'test_group': { 64: { - 'cost': decimal.Decimal('0.03'), + 'cost': decimal.Decimal( + '0.0299999999999999988897769754'), 'type': 'flat'}, 128: { - 'cost': decimal.Decimal('0.03'), + 'cost': decimal.Decimal( + '0.0299999999999999988897769754'), 'type': 'flat'}}}}}, 'mappings': { '_DEFAULT_': { - 'cost': decimal.Decimal('1.42'), + 'cost': decimal.Decimal( + '1.4199999999999999289457264240'), 'type': 'rate'}}, 'thresholds': {}}} self.assertEqual(expect, @@ -817,11 +824,11 @@ class HashMapRatingTest(tests.TestCase): expected_result = { '_DEFAULT_': { 'm1.tiny': { - 'cost': decimal.Decimal('1.337'), + 'cost': decimal.Decimal('1.3369999999999999662492200514'), 'type': 'flat'}}, 'test_group': { 'm1.large': { - 'cost': decimal.Decimal('13.37'), + 'cost': decimal.Decimal('13.3699999999999992184029906639'), 'type': 'rate'}}} self.assertEqual(expected_result, result) @@ -844,7 +851,7 @@ class HashMapRatingTest(tests.TestCase): expected_result = { 'test_group': { 1000: { - 'cost': decimal.Decimal('3.1337'), + 'cost': decimal.Decimal('3.1337000000000001520561454527'), 'type': 'flat'}}} self.assertEqual(expected_result, result) @@ -873,10 +880,14 @@ class HashMapRatingTest(tests.TestCase): actual_data = [actual_data] df_dicts = [d.as_dict(mutable=True) for d in expected_data] compute_list = df_dicts[0]['usage']['compute'] - compute_list[0]['rating'] = {'price': decimal.Decimal('2.757')} - compute_list[1]['rating'] = {'price': decimal.Decimal('5.514')} - compute_list[2]['rating'] = {'price': decimal.Decimal('5.514')} - compute_list[3]['rating'] = {'price': decimal.Decimal('2.757')} + compute_list[0]['rating'] = {'price': decimal.Decimal( + '2.756999999999999895194946475')} + compute_list[1]['rating'] = {'price': decimal.Decimal( + '5.513999999999999790389892950')} + compute_list[2]['rating'] = {'price': decimal.Decimal( + '5.513999999999999790389892950')} + compute_list[3]['rating'] = {'price': decimal.Decimal( + '2.756999999999999895194946475')} self.assertEqual(df_dicts, [d.as_dict(mutable=True) for d in actual_data]) @@ -917,10 +928,13 @@ class HashMapRatingTest(tests.TestCase): actual_data = [actual_data] df_dicts = [d.as_dict(mutable=True) for d in expected_data] compute_list = df_dicts[0]['usage']['compute'] - compute_list[0]['rating'] = {'price': decimal.Decimal('1.337')} - compute_list[1]['rating'] = {'price': decimal.Decimal('2.84')} + compute_list[0]['rating'] = {'price': decimal.Decimal( + '1.336999999999999966249220051')} + compute_list[1]['rating'] = {'price': decimal.Decimal( + '2.839999999999999857891452848')} compute_list[2]['rating'] = {'price': decimal.Decimal('0')} - compute_list[3]['rating'] = {'price': decimal.Decimal('1.47070')} + compute_list[3]['rating'] = {'price': decimal.Decimal( + '1.470700000000000081623596770')} self.assertEqual(df_dicts, [d.as_dict(mutable=True) for d in actual_data]) @@ -975,10 +989,14 @@ class HashMapRatingTest(tests.TestCase): actual_data = [actual_data] df_dicts = [d.as_dict(mutable=True) for d in expected_data] compute_list = df_dicts[0]['usage']['compute'] - compute_list[0]['rating'] = {'price': decimal.Decimal('0.1337')} - compute_list[1]['rating'] = {'price': decimal.Decimal('0.4')} - compute_list[2]['rating'] = {'price': decimal.Decimal('0.4')} - compute_list[3]['rating'] = {'price': decimal.Decimal('0.1337')} + compute_list[0]['rating'] = {'price': decimal.Decimal( + '0.1337000000000000132782673745')} + compute_list[1]['rating'] = {'price': decimal.Decimal( + '0.4000000000000000222044604926')} + compute_list[2]['rating'] = {'price': decimal.Decimal( + '0.4000000000000000222044604926')} + compute_list[3]['rating'] = {'price': decimal.Decimal( + '0.1337000000000000132782673745')} self.assertEqual(df_dicts, [d.as_dict(mutable=True) for d in actual_data]) @@ -1030,10 +1048,14 @@ class HashMapRatingTest(tests.TestCase): actual_data = [actual_data] df_dicts = [d.as_dict(mutable=True) for d in expected_data] compute_list = df_dicts[0]['usage']['compute'] - compute_list[0]['rating'] = {'price': decimal.Decimal('0.1')} - compute_list[1]['rating'] = {'price': decimal.Decimal('0.15')} - compute_list[2]['rating'] = {'price': decimal.Decimal('0.15')} - compute_list[3]['rating'] = {'price': decimal.Decimal('0.1')} + compute_list[0]['rating'] = {'price': decimal.Decimal( + '0.1000000000000000055511151231')} + compute_list[1]['rating'] = {'price': decimal.Decimal( + '0.1499999999999999944488848769')} + compute_list[2]['rating'] = {'price': decimal.Decimal( + '0.1499999999999999944488848769')} + compute_list[3]['rating'] = {'price': decimal.Decimal( + '0.1000000000000000055511151231')} self.assertEqual(df_dicts, [d.as_dict(mutable=True) for d in actual_data]) @@ -1175,11 +1197,15 @@ class HashMapRatingTest(tests.TestCase): end=expected_data[0].end) df_dicts = [d.as_dict(mutable=True) for d in expected_data] compute_list = df_dicts[0]['usage']['compute'] - compute_list[0]['rating'] = {'price': decimal.Decimal('2.487')} - compute_list[1]['rating'] = {'price': decimal.Decimal('5.564')} + compute_list[0]['rating'] = {'price': decimal.Decimal( + '2.486999999999999960698104928')} + compute_list[1]['rating'] = {'price': decimal.Decimal( + '5.564000000000000155875312656')} # 8vcpu mapping * 2 + service_mapping * 1 + 128m ram threshold * 2 - compute_list[2]['rating'] = {'price': decimal.Decimal('34.40')} - compute_list[3]['rating'] = {'price': decimal.Decimal('2.6357')} + compute_list[2]['rating'] = {'price': decimal.Decimal( + '34.40000000000000002220446049')} + compute_list[3]['rating'] = {'price': decimal.Decimal( + '2.635700000000000088840046430')} actual_data = [self._hash.process(d) for d in expected_data] self.assertEqual(df_dicts, [d.as_dict(mutable=True) for d in actual_data]) diff --git a/doc/source/user/rating/hashmap.rst b/doc/source/user/rating/hashmap.rst index 1a44ff35..efc28ba1 100644 --- a/doc/source/user/rating/hashmap.rst +++ b/doc/source/user/rating/hashmap.rst @@ -157,6 +157,12 @@ Apart from that, it works the same way as a mapping. As for mappings, a threshold can be tied to a specific scope/project. +Cost +---- +The cost option is the actual cost for the rating period. It has a precision of +28 decimal digits (on the right side of the number), and 30 digits on the left +side of the number. + Examples ========