Add config tests

opmon/__init__.py

@@ -9,7 +9,7 @@ class MonitoringPeriod(enum.Enum):
     DAY = "day"
 
 
-@attr.s(frozen=True, slots=True)
+@attr.s(auto_attribs=True)
 class DataSource:
     """Represents a table or view, from which Probes may be monitored.
     Args:
@@ -26,7 +26,7 @@ class DataSource:
     from_expression: str
 
 
-@attr.s(frozen=True, slots=True)
+@attr.s(auto_attribs=True)
 class Probe:
     """Represents a probe to be monitored."""
 
@@ -39,7 +39,7 @@ class Probe:
     type: Optional[str] = None
 
 
-@attr.s(frozen=True, slots=True)
+@attr.s(auto_attribs=True)
 class Dimension:
     """Represents a dimension for segmenting client populations."""
 

opmon/config.py

@@ -24,6 +24,7 @@ class DataSourceDefinition:
         return DataSource(**params)
 
 
+@attr.s(auto_attribs=True)
 class DataSourcesSpec:
     """Holds data source definitions.
 
@@ -47,6 +48,11 @@ class DataSourcesSpec:
         self.definitions.update(other.definitions)
 
 
+_converter.register_structure_hook(
+    DataSourcesSpec, lambda obj, _type: DataSourcesSpec.from_dict(obj)
+)
+
+
 @attr.s(auto_attribs=True)
 class DataSourceReference:
     name: str
@@ -58,6 +64,11 @@ class DataSourceReference:
         return spec.data_sources.definitions[self.name].resolve(spec)
 
 
+_converter.register_structure_hook(
+    DataSourceReference, lambda obj, _type: DataSourceReference(name=obj)
+)
+
+
 @attr.s(auto_attribs=True)
 class ProbeDefinition:
     """Describes the interface for defining a probe in configuration."""
@@ -82,6 +93,45 @@ class ProbeDefinition:
         )
 
 
+@attr.s(auto_attribs=True)
+class ProbesSpec:
+    """Describes the interface for defining custom probe definitions."""
+
+    definitions: Dict[str, ProbeDefinition] = attr.Factory(dict)
+
+    @classmethod
+    def from_dict(cls, d: dict) -> "ProbesSpec":
+        d = dict((k.lower(), v) for k, v in d.items())
+
+        definitions = {
+            k: _converter.structure({"name": k, **v}, ProbeDefinition) for k, v in d.items()
+        }
+        return cls(definitions=definitions)
+
+    def merge(self, other: "ProbesSpec"):
+        """
+        Merge another probe spec into the current one.
+        The `other` ProbesSpec overwrites existing keys.
+        """
+        self.definitions.update(other.definitions)
+
+
+_converter.register_structure_hook(ProbesSpec, lambda obj, _type: ProbesSpec.from_dict(obj))
+
+
+@attr.s(auto_attribs=True)
+class ProbeReference:
+    name: str
+
+    def resolve(self, spec: "MonitoringSpec") -> List[Probe]:
+        if self.name in spec.probes.definitions:
+            return spec.probes.definitions[self.name].resolve(spec)
+        raise ValueError(f"Could not locate probe {self.name}")
+
+
+_converter.register_structure_hook(ProbeReference, lambda obj, _type: ProbeReference(name=obj))
+
+
 @attr.s(auto_attribs=True)
 class DimensionDefinition:
     """Describes the interface for defining a dimension in configuration."""
@@ -102,22 +152,6 @@ class DimensionDefinition:
         )
 
 
-@attr.s(auto_attribs=True)
-class ProbesSpec:
-    """Describes the interface for defining custom probe definitions."""
-
-    definitions: Dict[str, ProbeDefinition] = attr.Factory(dict)
-
-    @classmethod
-    def from_dict(cls, d: dict) -> "ProbesSpec":
-        d = dict((k.lower(), v) for k, v in d.items())
-
-        definitions = {
-            k: _converter.structure({"name": k, **v}, ProbeDefinition) for k, v in d.items()
-        }
-        return cls(definitions=definitions)
-
-
 @attr.s(auto_attribs=True)
 class DimensionsSpec:
     """Describes the interface for defining custom dimensions."""
@@ -133,15 +167,15 @@ class DimensionsSpec:
         }
         return cls(definitions=definitions)
 
+    def merge(self, other: "DimensionsSpec"):
+        """
+        Merge another dimension spec into the current one.
+        The `other` DimensionsSpec overwrites existing keys.
+        """
+        self.definitions.update(other.definitions)
 
-@attr.s(auto_attribs=True)
-class ProbeReference:
-    name: str
 
-    def resolve(self, spec: "MonitoringSpec") -> List[Probe]:
-        if self.name in spec.probes.definitions:
-            return spec.probes.definitions[self.name].resolve(spec)
-        raise ValueError(f"Could not locate probe {self.name}")
+_converter.register_structure_hook(DimensionsSpec, lambda obj, _type: DimensionsSpec.from_dict(obj))
 
 
 @attr.s(auto_attribs=True)
@@ -154,6 +188,11 @@ class DimensionReference:
         raise ValueError(f"Could not locate dimension {self.name}")
 
 
+_converter.register_structure_hook(
+    DimensionReference, lambda obj, _type: DimensionReference(name=obj)
+)
+
+
 @attr.s(auto_attribs=True, kw_only=True)
 class PopulationConfiguration:
     data_source: Optional[DataSource] = None
@@ -179,6 +218,10 @@ class PopulationSpec:
             or ([branch.slug for branch in experiment.branches] if experiment else []),
         )
 
+    def merge(self, other: "PopulationSpec") -> None:
+        for key in attr.fields_dict(type(self)):
+            setattr(self, key, getattr(other, key) or getattr(self, key))
+
 
 @attr.s(auto_attribs=True, kw_only=True)
 class ProjectConfiguration:
@@ -203,12 +246,17 @@ def _parse_date(yyyy_mm_dd: Optional[str]) -> Optional[datetime]:
 @attr.s(auto_attribs=True, kw_only=True)
 class ProjectSpec:
     name: Optional[str] = None
-    xaxis: MonitoringPeriod = attr.ib(default=MonitoringPeriod.DAY)
+    xaxis: Optional[MonitoringPeriod] = None
     start_date: Optional[str] = attr.ib(default=None, validator=_validate_yyyy_mm_dd)
     end_date: Optional[str] = attr.ib(default=None, validator=_validate_yyyy_mm_dd)
     probes: List[ProbeReference] = attr.Factory(list)
     population: PopulationSpec = attr.Factory(PopulationSpec)
 
+    @classmethod
+    def from_dict(cls, d: dict) -> "ProjectSpec":
+        d = dict((k.lower(), v) for k, v in d.items())
+        return _converter.structure(d, cls)
+
     def resolve(
         self, spec: "MonitoringSpec", experiment: Optional[Experiment]
     ) -> ProjectConfiguration:
@@ -216,12 +264,21 @@ class ProjectSpec:
 
         return ProjectConfiguration(
             name=self.name or (experiment.name if experiment else None),
-            xaxis=self.xaxis,
-            start_date=self.start_date or (experiment.start_date if experiment else None),
-            end_date=self.end_date or (experiment.end_date if experiment else None),
+            xaxis=self.xaxis or MonitoringPeriod.DAY,
+            start_date=_parse_date(
+                self.start_date or (experiment.start_date if experiment else None)
+            ),
+            end_date=_parse_date(self.end_date or (experiment.end_date if experiment else None)),
             population=self.population.resolve(spec, experiment),
         )
 
+    def merge(self, other: "ProjectSpec") -> None:
+        for key in attr.fields_dict(type(self)):
+            if key == "population":
+                self.population.merge(other.population)
+            else:
+                setattr(self, key, getattr(other, key) or getattr(self, key))
+
 
 @attr.s(auto_attribs=True)
 class MonitoringConfiguration:
@@ -268,7 +325,7 @@ class MonitoringSpec:
 
         # filter to only have probes that actually need to be monitored
         probes = []
-        for probe_ref in self.project.probes:
+        for probe_ref in {p.name for p in self.project.probes}:
             if probe_ref in self.probes.definitions:
                 probes.append(self.probes.definitions[probe_ref].resolve(self))
             else:
@@ -276,7 +333,7 @@ class MonitoringSpec:
 
         # filter to only have dimensions that actually are in use
         dimensions = []
-        for dimension_ref in self.project.population.dimensions:
+        for dimension_ref in {d.name for d in self.project.population.dimensions}:
             if dimension_ref in self.dimensions.definitions:
                 dimensions.append(self.dimensions.definitions[dimension_ref].resolve(self))
             else:
@@ -287,3 +344,10 @@ class MonitoringSpec:
             probes=probes,
             dimensions=dimensions,
         )
+
+    def merge(self, other: "MonitoringSpec"):
+        """Merges another monitoring spec into the current one."""
+        self.project.merge(other.project)
+        self.data_sources.merge(other.data_sources)
+        self.probes.merge(other.probes)
+        self.dimensions.merge(other.dimensions)

opmon/tests/test_config.py

@@ -1,3 +1,11 @@
+from datetime import datetime
+from textwrap import dedent
+
+import pytest
+import pytz
+import toml
+
+from opmon import MonitoringPeriod
 from opmon.config import MonitoringConfiguration, MonitoringSpec
 
 
@@ -8,3 +16,225 @@ class TestConfig:
         cfg = spec.resolve()
         assert isinstance(cfg, MonitoringConfiguration)
         assert cfg.probes == []
+
+    def test_probe_definition(self):
+        config_str = dedent(
+            """
+            [project]
+            probes = ["test"]
+
+            [probes]
+            [probes.test]
+            select_expression = "SELECT 1"
+            data_source = "foo"
+
+            [data_sources]
+            [data_sources.foo]
+            from_expression = "test"
+            """
+        )
+        spec = MonitoringSpec.from_dict(toml.loads(config_str))
+        assert spec.probes.definitions["test"].select_expression == "SELECT 1"
+        assert spec.data_sources.definitions["foo"].from_expression == "test"
+        conf = spec.resolve()
+        assert conf.probes[0].name == "test"
+        assert conf.probes[0].data_source.name == "foo"
+
+    def test_duplicate_probes_are_okay(self, experiments):
+        config_str = dedent(
+            """
+            [project]
+            probes = ["test", "test"]
+
+            [probes]
+            [probes.test]
+            select_expression = "SELECT 1"
+            data_source = "foo"
+
+            [data_sources]
+            [data_sources.foo]
+            from_expression = "test"
+            """
+        )
+        spec = MonitoringSpec.from_dict(toml.loads(config_str))
+        cfg = spec.resolve()
+        assert len(cfg.probes) == 1
+
+    def test_data_source_definition(self, experiments):
+        config_str = dedent(
+            """
+            [project]
+            probes = ["test", "test2"]
+
+            [probes]
+            [probes.test]
+            select_expression = "SELECT 1"
+            data_source = "eggs"
+
+            [probes.test2]
+            select_expression = "SELECT 1"
+            data_source = "silly_knight"
+
+            [data_sources.eggs]
+            from_expression = "england.camelot"
+
+            [data_sources.silly_knight]
+            from_expression = "france"
+            """
+        )
+        spec = MonitoringSpec.from_dict(toml.loads(config_str))
+        cfg = spec.resolve()
+        test = [p for p in cfg.probes if p.name == "test"][0]
+        test2 = [p for p in cfg.probes if p.name == "test2"][0]
+        assert test.data_source.name == "eggs"
+        assert "camelot" in test.data_source.from_expression
+        assert test2.data_source.name == "silly_knight"
+        assert "france" in test2.data_source.from_expression
+
+    def test_merge(self, experiments):
+        """Test merging configs"""
+        config_str = dedent(
+            """
+            [probes]
+            [probes.test]
+            select_expression = "SELECT 1"
+            data_source = "foo"
+
+            [probes.test2]
+            select_expression = "SELECT 2"
+            data_source = "foo"
+
+            [data_sources]
+            [data_sources.foo]
+            from_expression = "test"
+
+            [dimensions]
+            [dimensions.foo]
+            select_expression = "bar"
+            data_source = "foo"
+            """
+        )
+        spec = MonitoringSpec.from_dict(toml.loads(config_str))
+
+        config_str = dedent(
+            """
+            [project]
+            name = "foo"
+            probes = ["test", "test2"]
+
+            [probes]
+            [probes.test]
+            select_expression = "SELECT 'd'"
+            data_source = "foo"
+            """
+        )
+        spec2 = MonitoringSpec.from_dict(toml.loads(config_str))
+        spec.merge(spec2)
+        cfg = spec.resolve()
+
+        assert cfg.project.name == "foo"
+        test = [p for p in cfg.probes if p.name == "test"][0]
+        test2 = [p for p in cfg.probes if p.name == "test2"][0]
+        assert test.select_expression == "SELECT 'd'"
+        assert test.data_source.name == "foo"
+        assert test2.select_expression == "SELECT 2"
+
+    def test_unknown_probe_failure(self, experiments):
+        config_str = dedent(
+            """
+            [project]
+            name = "foo"
+            probes = ["test", "test2"]
+
+            [probes]
+            [probes.test]
+            select_expression = "SELECT 'd'"
+            data_source = "foo"
+
+            [data_sources]
+            [data_sources.foo]
+            from_expression = "test"
+            """
+        )
+
+        with pytest.raises(ValueError) as e:
+            spec = MonitoringSpec.from_dict(toml.loads(config_str))
+            spec.resolve()
+
+        assert "No definition for probe test2." in str(e)
+
+    def test_overwrite_population(self):
+        config_str = dedent(
+            """
+            [project]
+            name = "foo"
+            xaxis = "build_id"
+            probes = []
+            start_date = "2022-01-01"
+            end_date = "2022-02-01"
+
+            [project.population]
+            data_source = "foo"
+            boolean_pref = "TRUE"
+            branches = ["treatment"]
+            dimensions = ["os"]
+
+            [data_sources]
+            [data_sources.foo]
+            from_expression = "test"
+
+            [dimensions]
+            [dimensions.os]
+            select_expression = "os"
+            data_source = "foo"
+            """
+        )
+
+        spec = MonitoringSpec.from_dict(toml.loads(config_str))
+
+        config_str = dedent(
+            """
+            [project]
+            name = "foo bar"
+            end_date = "2022-03-01"
+
+            [project.population]
+            boolean_pref = "FALSE"
+            branches = ["test-1"]
+            """
+        )
+
+        spec2 = MonitoringSpec.from_dict(toml.loads(config_str))
+        spec.merge(spec2)
+        cfg = spec.resolve()
+
+        assert cfg.project.name == "foo bar"
+        assert cfg.project.xaxis == MonitoringPeriod.BUILD_ID
+        assert cfg.project.start_date == datetime(2022, 1, 1, tzinfo=pytz.utc)
+        assert cfg.project.end_date == datetime(2022, 3, 1, tzinfo=pytz.utc)
+        assert cfg.project.population.data_source.name == "foo"
+        assert cfg.project.population.boolean_pref == "FALSE"
+        assert cfg.project.population.branches == ["test-1"]
+        assert len(cfg.dimensions) == 1
+
+    def test_bad_project_dates(self):
+        config_str = dedent(
+            """
+            [project]
+            start_date = "My birthday"
+            """
+        )
+
+        with pytest.raises(ValueError):
+            MonitoringSpec.from_dict(toml.loads(config_str))
+
+    def test_bad_project_xaxis(self):
+        config_str = dedent(
+            """
+            [project]
+            xaxis = "Nothing"
+            """
+        )
+
+        with pytest.raises(ValueError):
+            MonitoringSpec.from_dict(toml.loads(config_str))