diff --git a/cfretl/pingsource.py b/cfretl/pingsource.py index a7b7221..e0b3897 100644 --- a/cfretl/pingsource.py +++ b/cfretl/pingsource.py @@ -11,7 +11,6 @@ import datetime import pytz import contextlib -from cfretl.settings import CFR_VECTOR_WIDTH from jsonschema import validate LIMIT_CLAUSE = "" @@ -100,8 +99,8 @@ def validate_vector(self, vector): "properties": { "weights": { "type": "array", - "minItems": CFR_VECTOR_WIDTH, - "maxItems": CFR_VECTOR_WIDTH, + "minItems": 24, + "maxItems": 24, "items": {"type": "integer"}, } }, diff --git a/cfretl/remote_settings.py b/cfretl/remote_settings.py index 22076b5..86a2e4c 100644 --- a/cfretl/remote_settings.py +++ b/cfretl/remote_settings.py @@ -23,13 +23,15 @@ class SecurityError(Exception): pass +class CloneError(Exception): + pass + + class CFRRemoteSettings: """ This class can manage the Remote Settings server for CFR experimentation. - We only write to a single collection: 'cfr-experiment' - The other collections related to this experiment must be loaded manually and go through the regular dual sign off process. @@ -78,7 +80,7 @@ def _create_collection(self, id): auth = HTTPBasicAuth(self._kinto_user, self._kinto_pass) url = "{base_uri:s}/buckets/main/collections".format(base_uri=self._kinto_uri) status_code = requests.post( - url, json={"data": {"id": CFR_EXPERIMENT}}, auth=auth + url, json={"data": {"id": id}}, auth=auth ).status_code return status_code >= 200 and status_code < 300 @@ -91,18 +93,70 @@ def create_control_collection(self): def create_model_collection(self): return self._create_collection(CFR_MODELS) - def write_weights(self, json_data): + def _test_read_models(self): + """ + Read the model from RemoteSettings. This method is only used + for testing + """ + try: + url = "{base_uri:s}/buckets/main/collections/{c_id:s}/records/{c_id:s}".format( + base_uri=self._kinto_uri, c_id=CFR_MODELS + ) + resp = requests.get(url) + jdata = resp.json()["data"] + del jdata["id"] + del jdata["last_modified"] + except Exception: + # This method is only used for testing purposes - it's + # safe to just re-raise the exception here + raise + return jdata + + def write_models(self, json_data): jsonschema.validate(json_data, self.schema) - - if not self.check_experiment_exists(): - if not self.create_experiment_collection(): - raise SecurityError("CFR-Experiment collection could not be created.") + if not self.check_model_exists(): + if not self.create_model_collection(): + raise SecurityError("cfr-model collection could not be created.") auth = HTTPBasicAuth(self._kinto_user, self._kinto_pass) - url = "{base_uri:s}/buckets/main/collections/{id:s}".format( - base_uri=self._kinto_uri, id=CFR_EXPERIMENT + url = "{base_uri:s}/buckets/main/collections/{c_id:s}/records/{c_id:s}".format( + base_uri=self._kinto_uri, c_id=CFR_MODELS ) - resp = requests.put(url, json={"data": json_data}, auth=auth) + jdata = {"data": json_data} + resp = requests.put(url, json=jdata, auth=auth) return resp.status_code >= 200 and resp.status_code < 300 + + def cfr_records(self): + url = "{base_uri:s}/buckets/main/collections/{c_id:s}/records".format( + base_uri=self._kinto_uri, c_id="cfr" + ) + resp = requests.get(url) + jdata = resp.json()["data"] + cfr_records = jdata["data"] + return cfr_records + + def clone_to_cfr_control(self, cfr_data): + """ + Read the model from RemoteSettings. This method is only used + for testing + """ + + if not self.check_control_exists(): + if not self.create_control_collection(): + raise SecurityError("cfr-model collection could not be created.") + + auth = HTTPBasicAuth(self._kinto_user, self._kinto_pass) + + for obj in cfr_data: + # Extract the record ID so we can address it directly into + # the cfr-control bucket + obj_id = obj["id"] + + url = "{base_uri:s}/buckets/main/collections/{c_id:s}/records/{obj_id:s}".format( + base_uri=self._kinto_uri, c_id=CFR_CONTROL, obj_id=obj_id + ) + resp = requests.put(url, json={'data': obj}, auth=auth) + if resp.status_code > 299: + raise CloneError("Error cloning CFR record id: {}".format(obj_id)) diff --git a/cfretl/settings.py b/cfretl/settings.py index 29139c0..fe346b5 100644 --- a/cfretl/settings.py +++ b/cfretl/settings.py @@ -7,10 +7,8 @@ # Default CFR Vector width is 7 -CFR_VECTOR_WIDTH = config("CFR_VECTOR_WIDTH", 7) - KINTO_BUCKET = "main" -KINTO_URI = config("KINTO_URI", "http://localhost:8888/v1") -KINTO_USER = config("KINTO_USER", "admin") -KINTO_PASS = config("KINTO_PASS", "s3cr3t") +KINTO_URI = config("KINTO_URI", "https://kinto.dev.mozaws.net/v1") +KINTO_USER = config("KINTO_USER", "cfr-bot") +KINTO_PASS = config("KINTO_PASS", "botpass") diff --git a/scripts/bot_signoff.sh b/scripts/bot_signoff.sh index ecfbeba..903d85b 100644 --- a/scripts/bot_signoff.sh +++ b/scripts/bot_signoff.sh @@ -66,7 +66,7 @@ curl -X PATCH $SERVER/buckets/main/groups/cfr-control-reviewers \ # Generate some dummy data in the cfr-models bucket -curl -X PUT ${SERVER}/buckets/main-workspace/collections/cfr-experiment/records/5ba22305-6a9e-4f2b-beee-0a0a84d44708 \ +curl -X PUT ${SERVER}/buckets/main/collections/cfr-models/records/cfr-models \ -H 'Content-Type:application/json' \ -d "{\"data\": {\"property\": 321.1}}" \ -u ${BASIC_AUTH} --verbose diff --git a/tests/conftest.py b/tests/conftest.py index a5d8583..4b76415 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,12 +2,19 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. -import pytest from unittest.mock import MagicMock -from cfretl.pingsource import BQPingLoader import datetime +import json +import pytest import pytz +from cfretl.pingsource import BQPingLoader + +import os + +parent = os.path.split(__file__) +FIXTURE_PATH = os.path.join(os.path.split(__file__)[0], "fixtures") + @pytest.fixture def FIXTURE_JSON(): @@ -43,6 +50,13 @@ def bqpl(FIXTURE_JSON): bqpl.get_pings = MagicMock(return_value=[FIXTURE_JSON]) # Clobber the model generation - bqpl.compute_vector_weights = MagicMock(return_value={'weights': [2, 5, 9, 23, 5, 8, 12]}) + bqpl.compute_vector_weights = MagicMock( + return_value={"weights": [2, 5, 9, 23, 5, 8, 12]} + ) return bqpl + + +@pytest.fixture +def MOCK_CFR_DATA(): + return json.load(open(os.path.join(FIXTURE_PATH, "cfr.json")))['data'] diff --git a/tests/test_remote_settings.py b/tests/test_remote_settings.py index a4e718a..3a2be76 100644 --- a/tests/test_remote_settings.py +++ b/tests/test_remote_settings.py @@ -40,6 +40,17 @@ ] +def _compare_weights(expected, actual): + sorted_e_keys = sorted(expected.keys()) + sorted_a_keys = sorted(actual.keys()) + assert sorted_e_keys == sorted_a_keys + + sorted_e_weights = [expected[k] for k in sorted_e_keys] + sorted_a_weights = [actual[k] for k in sorted_e_keys] + + assert sorted_e_weights == sorted_a_weights + + @pytest.fixture def WEIGHT_VECTOR(): return dict(zip(CFR_IDS, [random.randint(0, 16000) for i in range(len(CFR_IDS))])) @@ -47,17 +58,28 @@ def WEIGHT_VECTOR(): def test_write_weights(WEIGHT_VECTOR): cfr_remote = CFRRemoteSettings() - assert cfr_remote.write_weights(WEIGHT_VECTOR) + assert cfr_remote.write_models(WEIGHT_VECTOR) + actual = cfr_remote._test_read_models() + _compare_weights(WEIGHT_VECTOR, actual) -@pytest.mark.skip -def test_clone_collection(): - pass +def test_update_weights(WEIGHT_VECTOR): + cfr_remote = CFRRemoteSettings() + # Pick a key + key = iter(WEIGHT_VECTOR.keys()).__next__() -@pytest.mark.skip -def test_update_collection(): - pass + for _ in range(3): + WEIGHT_VECTOR[key] += 1 + assert cfr_remote.write_models(WEIGHT_VECTOR) + + actual = cfr_remote._test_read_models() + _compare_weights(WEIGHT_VECTOR, actual) + + +def test_clone_into_cfr_control(MOCK_CFR_DATA): + cfr_remote = CFRRemoteSettings() + cfr_remote.clone_to_cfr_control(MOCK_CFR_DATA) @pytest.mark.skip