From 0360f3da4a3e86777fcf75deda06883651643250 Mon Sep 17 00:00:00 2001 From: Abhinav Upadhyay Date: Mon, 17 Dec 2018 15:33:39 +0530 Subject: [PATCH] Minor refactoring/cleanup of environment variables feature and tests --- apptuit/apptuit_client.py | 30 +++--- tests/test_token_and_tags.py | 196 +++++++++++++++++++---------------- 2 files changed, 118 insertions(+), 108 deletions(-) diff --git a/apptuit/apptuit_client.py b/apptuit/apptuit_client.py index a9c93ea..859fcbd 100644 --- a/apptuit/apptuit_client.py +++ b/apptuit/apptuit_client.py @@ -16,7 +16,7 @@ import requests import pandas as pd -APPTUIT_API_TOKEN = "APPTUIT_API_TOKEN" +APPTUIT_PY_TOKEN = "APPTUIT_PY_TOKEN" APPTUIT_PY_TAGS = "APPTUIT_PY_TAGS" VALID_CHARSET = set(ascii_letters + digits + "-_./") INVALID_CHARSET = frozenset(map(chr, range(128))) - VALID_CHARSET @@ -67,17 +67,17 @@ def _get_tags_from_environment(): tags_str = tags_str.strip(", ") tags_split = tags_str.split(',') for tag in tags_split: + tag = tag.strip() + if not tag: + continue try: - tag = tag.strip() - if not tag: - continue key, val = tag.split(":") tags[key.strip()] = val.strip() except ValueError: - raise ValueError("Invalid format of " + raise ValueError("Invalid format for " + APPTUIT_PY_TAGS + ", failed to parse tag key-value pair '" - + tag + "' format should be like" + + tag + "', " + APPTUIT_PY_TAGS + " should be in the format - " "'tag_key1:tag_val1,tag_key2:tag_val2,...,tag_keyN:tag_valN'") _validate_tags(tags) return tags @@ -96,23 +96,21 @@ class Apptuit(object): Apptuit is the client object, encapsulating the functionalities provided by Apptuit APIs """ - def __init__(self, token=None, api_endpoint="https://api.apptuit.ai/", debug=False): + def __init__(self, token=None, api_endpoint="https://api.apptuit.ai", debug=False): """ Creates an apptuit client object Params: token: Token of the tenant to which we wish to connect api_endpoint: Apptuit API End point (including the protocol and port) - port: Port on which the service is running - """ self.token = token if not self.token: - self.token = os.environ.get(APPTUIT_API_TOKEN) + self.token = os.environ.get(APPTUIT_PY_TOKEN) if not self.token: raise ValueError("Missing Apptuit API token, " "either pass it as a parameter or " "set as value of the environment variable '" - + APPTUIT_API_TOKEN + "'.") + + APPTUIT_PY_TOKEN + "'.") self.endpoint = api_endpoint if self.endpoint[-1] == '/': self.endpoint = self.endpoint[:-1] @@ -369,13 +367,12 @@ def tags(self): @tags.setter def tags(self, tags): + self._tags = None if tags is None: - self._tags = tags return - else: - if not isinstance(tags, dict): - raise ValueError("Expected a value of type dict for tags") - _validate_tags(tags) + if not isinstance(tags, dict): + raise ValueError("Expected a value of type dict for tags") + _validate_tags(tags) self._tags = tags @property @@ -386,7 +383,6 @@ def value(self): def value(self, value): if isinstance(value, (int, float)): self._value = value - elif isinstance(value, str): try: self._value = float(value) diff --git a/tests/test_token_and_tags.py b/tests/test_token_and_tags.py index bb35ae9..aa0a58c 100644 --- a/tests/test_token_and_tags.py +++ b/tests/test_token_and_tags.py @@ -1,13 +1,15 @@ """ -Tests on Apptuit class +Tests for token and global tags environment variables """ + +import math +import time import os from nose.tools import assert_equals, assert_raises from pyformance import MetricsRegistry -from apptuit.apptuit_client import APPTUIT_API_TOKEN, APPTUIT_PY_TAGS, _get_tags_from_environment - -from apptuit import Apptuit, DataPoint +from apptuit.apptuit_client import APPTUIT_PY_TOKEN, APPTUIT_PY_TAGS +from apptuit import apptuit_client, Apptuit, DataPoint from apptuit.pyformance import ApptuitReporter try: @@ -15,71 +17,61 @@ except ImportError: from mock import Mock, patch -def _tester_test_token_positive(token_environ, token_argument, token_test): - """ - to test test_token_positive - """ - mock_environ = patch.dict(os.environ, {APPTUIT_API_TOKEN: token_environ}) - mock_environ.start() - client = Apptuit(token=token_argument) - assert_equals(client.token, token_test) - mock_environ.stop() - def test_token_positive(): """ - Test that token is working normally - """ - _tester_test_token_positive("environ_token","","environ_token") - _tester_test_token_positive("environ_token", None, "environ_token") - _tester_test_token_positive("environ_token", "argument_token", "argument_token") - -def _get_tags_from_environ_negative(test_environ_str): - """ - to test test_get_tags_from_environ - """ - mock_environ = patch.dict(os.environ, { - APPTUIT_PY_TAGS: test_environ_str}) - mock_environ.start() - with assert_raises(ValueError): - _get_tags_from_environment() - mock_environ.stop() - -def _get_tags_from_environ_positive(test_tags_str,test_tags_dict): - """ - to test test_get_tags_from_environ - """ - mock_environ = patch.dict(os.environ, { - APPTUIT_PY_TAGS: test_tags_str}) - mock_environ.start() - assert_equals(_get_tags_from_environment(), test_tags_dict) - mock_environ.stop() - -def test_get_tags_from_environ(): - """ - Test that _get_tags_from_environment is working - """ - _get_tags_from_environ_positive(" ", {}) - _get_tags_from_environ_positive('tagk1: 22, tagk2: tagv2', - {"tagk1": "22", "tagk2": "tagv2"}) - _get_tags_from_environ_positive('tagk1: 22, , tagk2: tagv2', - {"tagk1": "22", "tagk2": "tagv2"}) - _get_tags_from_environ_positive(' tagk1 : 22,,tagk2 : tagv2 ', - {"tagk1": "22", "tagk2": "tagv2"}) - _get_tags_from_environ_positive(', , , , tagk1: 22, tagk2: tagv2, , , , ,', - {"tagk1": "22", "tagk2": "tagv2"}) - _get_tags_from_environ_positive(',tagk1: 22, tagk2: tagv2,', - {"tagk1": "22", "tagk2": "tagv2"}) - _get_tags_from_environ_negative('"tagk1":tagv1') - _get_tags_from_environ_negative('tagk1:tagv11:tagv12') - _get_tags_from_environ_negative('tag') - _get_tags_from_environ_negative(' tagk1 : 22,error,tagk2 : tagv2 ') - _get_tags_from_environ_negative(' tagk1 : 22,tagk1:tagv11:tagv12,tagk2 : tagv2 ') - - + Test various combinations of token argument to apptuit, as parameter and as env variable + """ + test_cases = [ + ("environ_token", "", "environ_token"), + ("environ_token", None, "environ_token"), + ("environ_token", "argument_token", "argument_token") + ] + for env_token, arg_token, expected in test_cases: + mock_environ = patch.dict(os.environ, {APPTUIT_PY_TOKEN: env_token}) + mock_environ.start() + client = Apptuit(token=arg_token) + assert_equals(client.token, expected) + mock_environ.stop() + +def test_tags_env_variable_parsing_negative(): + """ + Test that we fail when the value of global tags env variable is in an invalid format + """ + test_cases = [ + '"tagk1":tagv1', + 'tagk1:tagv11:tagv12', + 'tag', + ' tagk1 : 22,error,tagk2 : tagv2 ', + ' tagk1 : 22,tagk1:tagv11:tagv12,tagk2 : tagv2 ' + ] + for env_tags_value in test_cases: + mock_environ = patch.dict(os.environ, {APPTUIT_PY_TAGS: env_tags_value}) + mock_environ.start() + with assert_raises(ValueError): + apptuit_client._get_tags_from_environment() + mock_environ.stop() + +def test_tags_env_variable_parsing_positive(): + """ + Test that wel are able to parse the global tags from environment variable + """ + test_cases = [ + (" ", {}), + ('tagk1: 22, tagk2: tagv2', {"tagk1": "22", "tagk2": "tagv2"}), + ('tagk1: 22, , tagk2: tagv2', {"tagk1": "22", "tagk2": "tagv2"}), + (' tagk1 : 22,,tagk2 : tagv2 ', {"tagk1": "22", "tagk2": "tagv2"}), + (',tagk1: 22, tagk2: tagv2,', {"tagk1": "22", "tagk2": "tagv2"}), + (', , , , tagk1: 22, tagk2: tagv2, , , , ,', {"tagk1": "22", "tagk2": "tagv2"}) + ] + for env_tags_value, expected_global_tags in test_cases: + mock_environ = patch.dict(os.environ, {APPTUIT_PY_TAGS: env_tags_value}) + mock_environ.start() + assert_equals(apptuit_client._get_tags_from_environment(), expected_global_tags) + mock_environ.stop() def test_token_negative(): """ - Test that invalid token raises error + Test that if no token parameter is passed and no env variable is set, we get an error """ mock_environ = patch.dict(os.environ, {}) mock_environ.start() @@ -91,71 +83,81 @@ def test_token_negative(): Apptuit(token=None) mock_environ.stop() -def test_tags_positive(): +def test_env_global_tags_positive(): """ - Test that tags work normally + Test that apptuit works with global tags env variable and without them """ - mock_environ = patch.dict(os.environ, {APPTUIT_API_TOKEN: "environ_token", + mock_environ = patch.dict(os.environ, {APPTUIT_PY_TOKEN: "environ_token", APPTUIT_PY_TAGS: 'tagk1: 22, tagk2: tagv2'}) mock_environ.start() client = Apptuit() assert_equals(client._environ_tags, {"tagk1": "22", "tagk2": "tagv2"}) mock_environ.stop() -def test_tags_negative(): - """ - Test that invalid tag raises error - """ - mock_environ = patch.dict(os.environ, {APPTUIT_API_TOKEN: "environ_token"}) + mock_environ = patch.dict(os.environ, {APPTUIT_PY_TOKEN: "environ_token"}) mock_environ.start() client = Apptuit() assert_equals({}, client._environ_tags) mock_environ.stop() - mock_environ = patch.dict(os.environ, {APPTUIT_API_TOKEN: "environ_token", APPTUIT_PY_TAGS: "{InvalidTags"}) + +def test_env_global_tags_negative(): + """ + Negative test cases for global tags env variable + """ + mock_environ = patch.dict(os.environ, {APPTUIT_PY_TOKEN: "environ_token", + APPTUIT_PY_TAGS: "{InvalidTags"}) mock_environ.start() with assert_raises(ValueError): Apptuit() mock_environ.stop() - mock_environ = patch.dict(os.environ, {APPTUIT_API_TOKEN: "environ_token", APPTUIT_PY_TAGS: '"tagk1":"tagv1"'}) + mock_environ = patch.dict(os.environ, {APPTUIT_PY_TOKEN: "environ_token", + APPTUIT_PY_TAGS: '"tagk1":"tagv1"'}) mock_environ.start() with assert_raises(ValueError): Apptuit() mock_environ.stop() -def test_datapoint_tags_take_priority(): +def test_datapoint_tags_and_envtags(): """ - Test that datapoint tags take priority + Test that datapoint tags take priority when global tags env variable is present """ - mock_environ = patch.dict(os.environ, {APPTUIT_API_TOKEN: "environ_token", + mock_environ = patch.dict(os.environ, {APPTUIT_PY_TOKEN: "environ_token", APPTUIT_PY_TAGS: 'host: host1, ip: 1.1.1.1'}) mock_environ.start() client = Apptuit() - test_val = 123 - dp1 = DataPoint("test_metric", {"host": "host2", "ip": "2.2.2.2", "test": 1}, test_val, test_val) - dp2 = DataPoint("test_metric", {"test": 2}, test_val, test_val) - dp3 = DataPoint("test_metric", {}, test_val, test_val) - payload = client._create_payload([dp1, dp2, dp3]) - assert_equals(len(payload), 3) + timestamp = int(time.time()) + test_val = math.pi + dp1 = DataPoint("test_metric", {"host": "host2", "ip": "2.2.2.2", "test": 1}, + timestamp, test_val) + dp2 = DataPoint("test_metric", {"test": 2}, timestamp, test_val) + dp3 = DataPoint("test_metric", {}, timestamp, test_val) + dp4 = DataPoint("test_metric", None, timestamp, test_val) + payload = client._create_payload([dp1, dp2, dp3, dp4]) + assert_equals(len(payload), 4) assert_equals(payload[0]["tags"], {"host": "host2", "ip": "2.2.2.2", "test": 1}) assert_equals(payload[1]["tags"], {"host": "host1", "ip": "1.1.1.1", "test": 2}) assert_equals(payload[2]["tags"], {"host": "host1", "ip": "1.1.1.1"}) + assert_equals(payload[3]["tags"], {"host": "host1", "ip": "1.1.1.1"}) assert_equals(client._environ_tags, {"host": "host1", "ip": "1.1.1.1"}) mock_environ.stop() def test_no_environ_tags(): """ - Test No Environ tags work + Test tags work even if no global tags present as env variable """ - test_val = 123 - mock_environ = patch.dict(os.environ, {APPTUIT_API_TOKEN: "environ_token"}) + timestamp = int(time.time()) + test_val = math.pi + mock_environ = patch.dict(os.environ, {APPTUIT_PY_TOKEN: "environ_token"}) mock_environ.start() client = Apptuit() - dp1 = DataPoint("test_metric", {"host": "host2", "ip": "2.2.2.2", "test": 1}, test_val, test_val) - dp2 = DataPoint("test_metric", {"test": 2}, test_val, test_val) + dp1 = DataPoint("test_metric", {"host": "host2", "ip": "2.2.2.2", "test": 1}, + timestamp, test_val) + dp2 = DataPoint("test_metric", {"test": 2}, timestamp, test_val) payload = client._create_payload([dp1, dp2]) assert_equals(len(payload), 2) assert_equals(payload[0]["tags"], {"host": "host2", "ip": "2.2.2.2", "test": 1}) assert_equals(payload[1]["tags"], {"test": 2}) + registry = MetricsRegistry() counter = registry.counter("counter") counter.inc(1) @@ -165,11 +167,23 @@ def test_no_environ_tags(): assert_equals(payload[0]["tags"], {'host': 'reporter', 'ip': '2.2.2.2'}) mock_environ.stop() -def test_reporter_tags_take_priority(): +def test_reporter_tags_with_global_env_tags(): """ Test that reporter tags take priority + TODO: + We have 8 possible combinations - + 1. global env tags: true, reporter tags: true, metric tags: true + 2. global env tags: true, reporter tags: true, metric tags: false + 3. global env tags: true, reporter tags: false, metric tags: true + 4. global env tags: true, reporter tags: false, metric tags: false + 5. global env tags: false, reporter tags: true, metric tags: true + 6. global env tags: false, reporter tags: true, metric tags: false + 7. global env tags: false, reporter tags: false, metric tags: true + 8. global env tags: false, reporter tags: false, metric tags: false """ - mock_environ = patch.dict(os.environ, {APPTUIT_API_TOKEN: "environ_token", + + + mock_environ = patch.dict(os.environ, {APPTUIT_PY_TOKEN: "environ_token", APPTUIT_PY_TAGS: 'host: environ, ip: 1.1.1.1'}) mock_environ.start() registry = MetricsRegistry() @@ -189,7 +203,7 @@ def test_tags_of_metric_take_priority(): """ Test that metric tags take priority """ - mock_environ = patch.dict(os.environ, {APPTUIT_API_TOKEN: "environ_token", + mock_environ = patch.dict(os.environ, {APPTUIT_PY_TOKEN: "environ_token", APPTUIT_PY_TAGS: 'host: environ, ip: 1.1.1.1'}) mock_environ.start() registry = MetricsRegistry()