Repository: ApptuitAI/apptuit-py

Issue:
DataPoint should validate parameters
Right now DataPoint does not validate if the "value" parameter is int/long or float. Eventual API call fails if the value is a string (even representation of int/float). 

DataPoint should perform client side validation of all input parameters (metricname, tags, values) without waiting for an error from the server.

As a bonus, if the value is a string representation of int/float, we could coerce it into a number instead of erroring out.

Discussion:

Patch:
diff --git a/apptuit/apptuit_client.py b/apptuit/apptuit_client.py
index afa6792..2049aa9 100644
--- a/apptuit/apptuit_client.py
+++ b/apptuit/apptuit_client.py
@@ -286,6 +286,23 @@ class DataPoint(object):
                 raise ValueError("Tag value %s contains an invalid character, allowed characters are a-z, A-Z, 0-9, -, _, ., and /" % tagv)
         self._tags = tags
 
+    @property
+    def value(self):
+        return self._value
+
+    @value.setter
+    def value(self, value):
+        if isinstance(value, (int, float)):
+            self._value = value
+
+        elif isinstance(value, str):
+            try:
+                self._value = float(value)
+            except ValueError:
+                raise ValueError("Expected a numeric value got %s" % value)
+        else:
+            raise ValueError("Expected a numeric value for the value parameter")
+
     def __repr__(self):
         repr = self.metric + "{"
         for tagk, tagv in self.tags.items():


Test patch:
diff --git a/tests/test_send.py b/tests/test_send.py
index f0d9b49..f8ee9a5 100644
--- a/tests/test_send.py
+++ b/tests/test_send.py
@@ -5,7 +5,7 @@ try:
 except ImportError:
     from mock import Mock, patch
 
-from nose.tools import assert_raises, ok_, raises
+from nose.tools import assert_raises, ok_, assert_is_not_none, assert_equals
 from apptuit import Apptuit, DataPoint, ApptuitException
 
 @patch('apptuit.apptuit_client.requests.post')
@@ -85,4 +85,60 @@ def test_invalid_metric_name():
     tags = {"host": "localhost", "region": "us-east-1", "service": "web-server"}
     ts = int(time.time())
     with assert_raises(ValueError) as ex:
-        DataPoint(metric_name, tags, ts, random.random())
\ No newline at end of file
+        DataPoint(metric_name, tags, ts, random.random())
+
+def test_invalid_datapoint_value():
+    """
+    Test for a non-numeric value for DataPoint
+    """
+    metric_name = "node.load.avg.1m"
+    tags = {"host": "localhost", "region": "us-east-1", "service": "web-server"}
+    ts = int(time.time())
+    value = 'abc'
+    with assert_raises(ValueError) as ex:
+        DataPoint(metric_name, tags, ts, value)
+
+def test_numeric_datapoint_value():
+    """
+    Test for a numeric DataPoint value
+    """
+    metric_name = "node.load.avg.1m"
+    tags = {"host": "localhost", "region": "us-east-1", "service": "web-server"}
+    ts = int(time.time())
+    value = 3.14
+    point = DataPoint(metric_name, tags, ts, value)
+    assert_is_not_none(point)
+
+def test_numeric_string_datapoint_value():
+    """
+    Test for a valid DataPoint value which is a string
+    """
+    metric_name = "node.load.avg.1m"
+    tags = {"host": "localhost", "region": "us-east-1", "service": "web-server"}
+    ts = int(time.time())
+    value = '3.14'
+    point = DataPoint(metric_name, tags, ts, value)
+    assert_is_not_none(point)
+
+def test_datapoint_value_getter():
+    """
+    Test that the value used to create the DataPoint matches with the value
+    returned by the object after creation
+    """
+    metric_name = "node.load.avg.1m"
+    tags = {"host": "localhost", "region": "us-east-1", "service": "web-server"}
+    ts = int(time.time())
+    value = 3.14
+    point = DataPoint(metric_name, tags, ts, value)
+    assert_equals(point.value, value)
+
+def test_nonstring_invalid_datapoint_value():
+    """
+    Test for a non-str/numeric value for datapoint value
+    """
+    metric_name = "node.load.avg.1m"
+    tags = {"host": "localhost", "region": "us-east-1", "service": "web-server"}
+    ts = int(time.time())
+    value = object()
+    with assert_raises(ValueError):
+        DataPoint(metric_name, tags, ts, value)