From 6ba9d45ea69ea308ae21052e45a8d114c39145da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Mon, 8 Feb 2021 13:58:14 -0300 Subject: [PATCH] Introduce "response_format" for the V2 summary API The V2 summary endpoint uses a quite unconventional data format in the response. Currently, the format is the following: ``` {"total": , "results": [array of arrays of data], "columns": [array of columns]} ``` To process this, we need to find the index of a column in the column list, and with this index, we retrieve the data in the array of data that is found in the array of results. The proposal is to use the following format in the response. ``` {"total": , "results": [array of objects/dictionary]} ``` With this new format, one does not need to consult the index of a column to retrieve data in one of the entries. We would only need to retrieve the data in the entry using its column name. Therefore, the coding feels more natural. To maintain compatibility, this new format would be only applied when an option is sent to CloudKitty via `response_format` option. Depends-on: https://review.opendev.org/c/openstack/cloudkitty/+/793973 Change-Id: I5869d527e6e4655c653b6852d6fb7bebc9d71520 --- cloudkitty/api/v2/summary/summary.py | 45 ++++++++++++++----- cloudkitty/storage/v2/influx.py | 4 +- .../tests/api/v2/summary/test_summary.py | 43 ++++++++++++++++++ .../api-reference/v2/summary/summary.inc | 22 ++++++++- .../v2/summary/summary_parameters.yml | 22 +++++++++ ...ormat-v2-summary-api-270facdb01d9202b.yaml | 5 +++ 6 files changed, 129 insertions(+), 12 deletions(-) create mode 100644 releasenotes/notes/response_format-v2-summary-api-270facdb01d9202b.yaml diff --git a/cloudkitty/api/v2/summary/summary.py b/cloudkitty/api/v2/summary/summary.py index 4bb17830..0d871f7e 100644 --- a/cloudkitty/api/v2/summary/summary.py +++ b/cloudkitty/api/v2/summary/summary.py @@ -20,12 +20,19 @@ from cloudkitty.api.v2 import utils as api_utils from cloudkitty.common import policy from cloudkitty.utils import tz as tzutils +TABLE_RESPONSE_FORMAT = "table" +OBJECT_RESPONSE_FORMAT = "object" + +ALL_RESPONSE_FORMATS = [TABLE_RESPONSE_FORMAT, OBJECT_RESPONSE_FORMAT] + class Summary(base.BaseResource): """Resource allowing to retrieve a rating summary.""" @api_utils.paginated @api_utils.add_input_schema('query', { + voluptuous.Optional('response_format'): + api_utils.SingleQueryParam(str), voluptuous.Optional('custom_fields'): api_utils.SingleQueryParam(str), voluptuous.Optional('groupby'): api_utils.MultiQueryParam(str), voluptuous.Optional('filters'): @@ -35,8 +42,15 @@ class Summary(base.BaseResource): voluptuous.Optional('end'): api_utils.SingleQueryParam( tzutils.dt_from_iso), }) - def get(self, custom_fields=None, groupby=None, filters={}, begin=None, - end=None, offset=0, limit=100): + def get(self, response_format=TABLE_RESPONSE_FORMAT, custom_fields=None, + groupby=None, filters={}, begin=None, end=None, offset=0, + limit=100): + + if response_format not in ALL_RESPONSE_FORMATS: + raise voluptuous.Invalid("Invalid response format [%s]. Valid " + "format are [%s]." + % (response_format, ALL_RESPONSE_FORMATS)) + policy.authorize( flask.request.context, 'summary:get_summary', @@ -69,12 +83,23 @@ class Summary(base.BaseResource): arguments['custom_fields'] = custom_fields total = self._storage.total(**arguments) - columns = [] - if len(total['results']) > 0: - columns = list(total['results'][0].keys()) - return { - 'total': total['total'], - 'columns': columns, - 'results': [list(res.values()) for res in total['results']] - } + return self.generate_response(response_format, total) + + @staticmethod + def generate_response(response_format, total): + response = {'total': total['total']} + if response_format == TABLE_RESPONSE_FORMAT: + columns = [] + if len(total['results']) > 0: + columns = list(total['results'][0].keys()) + + response['columns'] = columns + response['results'] = [list(res.values()) + for res in total['results']] + + elif response_format == OBJECT_RESPONSE_FORMAT: + response['results'] = total['results'] + + response['format'] = response_format + return response diff --git a/cloudkitty/storage/v2/influx.py b/cloudkitty/storage/v2/influx.py index 8fc058bf..22e057c3 100644 --- a/cloudkitty/storage/v2/influx.py +++ b/cloudkitty/storage/v2/influx.py @@ -191,7 +191,9 @@ class InfluxClient(object): self.validate_custom_fields(custom_fields) - query = 'SELECT %s FROM "dataframes"' % custom_fields + # We validate the SQL statements. Therefore, we can ignore this + # bandit warning here. + query = 'SELECT %s FROM "dataframes"' % custom_fields # nosec query += self._get_time_query(begin, end) query += self._get_filter_query(filters) query += self._get_type_query(types) diff --git a/cloudkitty/tests/api/v2/summary/test_summary.py b/cloudkitty/tests/api/v2/summary/test_summary.py index 9c0413f8..a1af3bd5 100644 --- a/cloudkitty/tests/api/v2/summary/test_summary.py +++ b/cloudkitty/tests/api/v2/summary/test_summary.py @@ -16,6 +16,7 @@ import flask import uuid from unittest import mock +import voluptuous from cloudkitty.api.v2.summary import summary from cloudkitty import tests @@ -52,3 +53,45 @@ class TestSummaryEndpoint(tests.TestCase): limit=100, paginate=True, ) + + def test_invalid_response_type(self): + self.assertRaises(voluptuous.Invalid, self.endpoint.get, + response_format="INVALID_RESPONSE_TYPE") + + def test_generate_response_table_response_type(self): + objects = [{"a1": "obj1", "a2": "value1"}, + {"a1": "obj2", "a2": "value2"}] + + total = {'total': len(objects), + 'results': objects} + + response = self.endpoint.generate_response( + summary.TABLE_RESPONSE_FORMAT, total) + + self.assertIn('total', response) + self.assertIn('results', response) + self.assertIn('columns', response) + + self.assertEqual(len(objects), response['total']) + self.assertEqual(list(objects[0].keys()), response['columns']) + self.assertEqual( + [list(res.values()) for res in objects], response['results']) + self.assertEqual(summary.TABLE_RESPONSE_FORMAT, response['format']) + + def test_generate_response_object_response_type(self): + objects = [{"a1": "obj1", "a2": "value1"}, + {"a1": "obj2", "a2": "value2"}] + + total = {'total': len(objects), + 'results': objects} + + response = self.endpoint.generate_response( + summary.OBJECT_RESPONSE_FORMAT, total) + + self.assertIn('total', response) + self.assertIn('results', response) + self.assertNotIn('columns', response) + + self.assertEqual(len(objects), response['total']) + self.assertEqual(objects, response['results']) + self.assertEqual(summary.OBJECT_RESPONSE_FORMAT, response['format']) diff --git a/doc/source/api-reference/v2/summary/summary.inc b/doc/source/api-reference/v2/summary/summary.inc index 84e71246..69ad4a03 100644 --- a/doc/source/api-reference/v2/summary/summary.inc +++ b/doc/source/api-reference/v2/summary/summary.inc @@ -18,6 +18,7 @@ Get a rating summary for one or several tenants. - groupby: groupby - filters: filters - custom_fields: custom_fields + - response_format: response_format Status codes ------------ @@ -35,7 +36,7 @@ Status codes Response -------- -The response has the following format: +The response has the following default format (response_format='table'): .. code-block:: javascript @@ -66,6 +67,25 @@ the columns for each element of ``results``. The columns are the four mandatory (``begin``, ``end``, ``qty``, ``rate``) along with each attribute the result is grouped by. +``format`` is the response format. It can be "table" or "object". The default +response structure is "table", which is presented above. The object structure +uses the following pattern. + +.. code-block:: javascript + + { + "results": [ + {"begin": "2019-06-01T00:00:00Z", + "end": "2019-07-01T00:00:00Z", + "qty": 2590.421676635742, + "rate": 1295.210838317871, + "group1": "group1", + "group2": "group2", + }, + ], + "total": 4 + } + .. note:: It is also possible to group data by time, in order to obtain timeseries. In order to do this, group by ``time``. No extra column will be added, but you'll get one entry per collect period in the queried timeframe. diff --git a/doc/source/api-reference/v2/summary/summary_parameters.yml b/doc/source/api-reference/v2/summary/summary_parameters.yml index f4fa8d5a..c2c2f0d9 100644 --- a/doc/source/api-reference/v2/summary/summary_parameters.yml +++ b/doc/source/api-reference/v2/summary/summary_parameters.yml @@ -64,6 +64,28 @@ offset: &offset type: int required: false +response_format: + in: query + description: | + Optional attribute to define the object structure used in the response. + Both responses will be JSON objects. Possible values are ``table`` or + ``object``. + + The default value is ``table`` object structure, where one has the + attributes `total`, which indicates the total number of entries in the + response; `results`, which is a list of lists, where the nested list + contains the values of each entry; and, `columns`, which is the attribute + that describes all of the available columns. Then, each index in this + list (`columns`) corresponds to the metadata of the values in the `results` + list. + + The structure for the `object` option uses a dictionary. The response still + has the `total` attribute. However, in the `results` attribute, one will + find a list of objects, instead of a list of lists of values that we see + in the `table` option. This facilitates the processing of some use cases. + type: string + required: false + begin_resp: <<: *begin required: true diff --git a/releasenotes/notes/response_format-v2-summary-api-270facdb01d9202b.yaml b/releasenotes/notes/response_format-v2-summary-api-270facdb01d9202b.yaml new file mode 100644 index 00000000..54a5732a --- /dev/null +++ b/releasenotes/notes/response_format-v2-summary-api-270facdb01d9202b.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Introduce ``response_format`` option for the V2 summary API, which can + facilitate parsing the response. \ No newline at end of file