Skip to content

Commit

Permalink
feat(nimbus): Integrate segment retrieval function from metric-config…
Browse files Browse the repository at this point in the history
…-parser

Because

* experimenter was manually parsing segments when it could just import them from metric-config-parser

This commit

* replaces manual segment parsing with metric-config-parser’s retrieval function

Fixes #11623
  • Loading branch information
RJAK11 committed Nov 1, 2024
1 parent 872da41 commit 0dede87
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 10 deletions.
34 changes: 26 additions & 8 deletions experimenter/experimenter/segments/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from dataclasses import dataclass

from django.core.checks import Error, register
from metric_config_parser.config import ConfigCollection
from metric_config_parser.segment import SegmentDefinition

from experimenter.experiments.constants import NimbusConstants

Expand Down Expand Up @@ -44,15 +46,20 @@ def _load_segments(cls, segment_data=None):
app_segments = segment_data.get(app_name, [])

for segment in app_segments:
segments.append(
Segment(
slug=segment.name,
friendly_name=segment.friendly_name,
application=app_config.slug,
description=segment.description,
select_expression=segment.select_expression,
if isinstance(segment, SegmentDefinition):
segments.append(
Segment(
slug=segment.name,
friendly_name=segment.friendly_name,
application=app_config.slug,
description=segment.description,
select_expression=segment.select_expression,
)
)
else:
raise TypeError(
f"Expected SegmentDefinition, got {type(segment).__name__}"
)
)

return segments

Expand All @@ -69,3 +76,14 @@ def all(cls, segment_data=None):
@classmethod
def by_application(cls, application, segment_data=None):
return [o for o in cls.all(segment_data) if o.application == application]


@register()
def check_segments(app_configs, segments_data=None, **kwargs):
errors = []

try:
Segments.all(segment_data=segments_data)
except Exception as e:
errors.append(Error(f"Error loading Segments: {e}"))
return errors
4 changes: 4 additions & 0 deletions experimenter/experimenter/segments/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,7 @@ def mock_get_segments():
),
],
}


def mock_get_invalid_segments():
return {"fenix": [{"test": "test_segment"}]}
23 changes: 21 additions & 2 deletions experimenter/experimenter/segments/tests/test_segments.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
from django.core.checks import Error
from django.test import TestCase

from experimenter.experiments.models import NimbusExperiment
from experimenter.segments import Segment, Segments
from experimenter.segments.tests import mock_get_segments
from experimenter.segments import Segment, Segments, check_segments
from experimenter.segments.tests import mock_get_invalid_segments, mock_get_segments


class TestSegments(TestCase):
Expand Down Expand Up @@ -109,3 +110,21 @@ def test_load_segments_by_application(self):
),
desktop_segments,
)


class TestCheckSegments(TestCase):
def setUp(self):
Segments.clear_cache()

def test_valid_segments_do_not_trigger_check_error(self):
errors = check_segments(app_configs=None, segments_data=mock_get_segments())
self.assertEqual(errors, [])

def test_invalid_segments_trigger_check_error(self):
errors = check_segments(
app_configs=None, segments_data=mock_get_invalid_segments()
)
self.assertEqual(
errors,
[Error(msg="Error loading Segments: Expected SegmentDefinition, got dict")],
)

0 comments on commit 0dede87

Please sign in to comment.