diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index d86c9c3..ca1b880 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -4,8 +4,22 @@ on: - pull_request jobs: - lint-unit-and-func-tests: - name: Lint, Unit, & Functional Tests + lint: + name: Lint + runs-on: ubuntu-latest + steps: + - name: Check out code + uses: actions/checkout@v2 + - name: Setup Python + uses: actions/setup-python@v2 + with: + python-version: 3.9 + - name: Install Tox + run: pip install tox + - name: Run lint + run: tox -e lint + unit-and-func-tests: + name: Unit, & Functional Tests runs-on: ubuntu-latest strategy: matrix: @@ -19,8 +33,8 @@ jobs: python-version: ${{ matrix.python }} - name: Install Tox run: pip install tox - - name: Run lint, unit, and functional tests - run: tox + - name: Run unit & functional tests + run: tox -e unit,functional # TODO #integration-test: diff --git a/loadbalancer_interface/base.py b/loadbalancer_interface/base.py index 60bb842..e513ff7 100644 --- a/loadbalancer_interface/base.py +++ b/loadbalancer_interface/base.py @@ -19,26 +19,30 @@ def __init__(self, charm, relation_name): # Future-proof against the need to evolve the relation protocol # by ensuring that we agree on a version number before starting. # This may or may not be made moot by a future feature in Juju. - for event in (charm.on[relation_name].relation_created, - charm.on.leader_elected, - charm.on.upgrade_charm): + for event in ( + charm.on[relation_name].relation_created, + charm.on.leader_elected, + charm.on.upgrade_charm, + ): self.framework.observe(event, self._set_version) def _set_version(self, event): if self.unit.is_leader(): - if hasattr(event, 'relation'): + if hasattr(event, "relation"): relations = [event.relation] else: relations = self.model.relations.get(self.relation_name, []) for relation in relations: - relation.data[self.app]['version'] = str(schemas.max_version) + relation.data[self.app]["version"] = str(schemas.max_version) @cached_property def relations(self): relations = self.model.relations.get(self.relation_name, []) - return [relation - for relation in sorted(relations, key=attrgetter('id')) - if self._schema(relation)] + return [ + relation + for relation in sorted(relations, key=attrgetter("id")) + if self._schema(relation) + ] def _schema(self, relation=None): if relation is None: @@ -46,9 +50,9 @@ def _schema(self, relation=None): if relation.app not in relation.data: return None data = relation.data[relation.app] - if 'version' not in data: + if "version" not in data: return None - remote_version = int(data['version']) + remote_version = int(data["version"]) return schemas.versions[min((schemas.max_version, remote_version))] @property diff --git a/loadbalancer_interface/consumer.py b/loadbalancer_interface/consumer.py index 7d3c18d..f3615b3 100644 --- a/loadbalancer_interface/consumer.py +++ b/loadbalancer_interface/consumer.py @@ -21,8 +21,8 @@ class LBConsumersEvents(ObjectEvents): class LBConsumers(VersionedInterface): - """ API used to interact with consumers of a loadbalancer provider. - """ + """API used to interact with consumers of a loadbalancer provider.""" + state = StoredState() on = LBConsumersEvents() @@ -31,9 +31,11 @@ def __init__(self, charm, relation_name): self.relation_name = relation_name self.state.set_default(known_requests={}) - for event in (charm.on[relation_name].relation_created, - charm.on[relation_name].relation_joined, - charm.on[relation_name].relation_changed): + for event in ( + charm.on[relation_name].relation_created, + charm.on[relation_name].relation_joined, + charm.on[relation_name].relation_changed, + ): self.framework.observe(event, self._check_consumers) def _check_consumers(self, event): @@ -42,8 +44,7 @@ def _check_consumers(self, event): @cached_property def all_requests(self): - """ A list of all current consumer requests. - """ + """A list of all current consumer requests.""" if not self.unit.is_leader(): # Only the leader can process requests, so avoid mistakes # by not even reading the requests if not the leader. @@ -54,17 +55,15 @@ def all_requests(self): local_data = relation.data[self.app] remote_data = relation.data[relation.app] for key, request_sdata in sorted(remote_data.items()): - if not key.startswith('request_'): + if not key.startswith("request_"): continue - name = key[len('request_'):] - response_sdata = local_data.get('response_' + name) - request = schema.Request.loads(name, - request_sdata, - response_sdata) + name = key[len("request_") :] + response_sdata = local_data.get("response_" + name) + request = schema.Request.loads(name, request_sdata, response_sdata) request.relation = relation if not request.backends: - for unit in sorted(relation.units, key=attrgetter('name')): - addr = relation.data[unit].get('ingress-address') + for unit in sorted(relation.units, key=attrgetter("name")): + addr = relation.data[unit].get("ingress-address") if addr: request.backends.append(addr) requests.append(request) @@ -73,44 +72,47 @@ def all_requests(self): @property def new_requests(self): - """ A list of requests with changes or no response. - """ - return [request for request in self.all_requests - if request.hash != self.state.known_requests[request.id]] + """A list of requests with changes or no response.""" + return [ + request + for request in self.all_requests + if request.hash != self.state.known_requests[request.id] + ] @property def removed_requests(self): - """ A list of requests which have been removed, either explicitly or + """A list of requests which have been removed, either explicitly or because the relation was removed. """ current_ids = {request.id for request in self.all_requests} unknown_ids = self.state.known_requests.keys() - current_ids schema = self._schema() - return [schema.Request._from_id(req_id, self.relations) - for req_id in sorted(unknown_ids)] + return [ + schema.Request._from_id(req_id, self.relations) + for req_id in sorted(unknown_ids) + ] def send_response(self, request): - """ Send a specific request's response. - """ + """Send a specific request's response.""" request.response.received_hash = request.sent_hash - key = 'response_' + request.name + key = "response_" + request.name request.relation.data[self.app][key] = request.response.dumps() self.state.known_requests[request.id] = request.hash if not self.new_requests: try: from charms.reactive import clear_flag - prefix = 'endpoint.' + self.relation_name - clear_flag(prefix + '.requests_changed') + + prefix = "endpoint." + self.relation_name + clear_flag(prefix + ".requests_changed") except ImportError: pass # not being used in a reactive charm def revoke_response(self, request): - """ Revoke / remove the response for a given request. - """ + """Revoke / remove the response for a given request.""" if request.id: self.state.known_requests.pop(request.id, None) if request.relation: - key = 'response_' + request.name + key = "response_" + request.name request.relation.data.get(self.app, {}).pop(key, None) @property @@ -118,8 +120,8 @@ def is_changed(self): return bool(self.new_requests or self.removed_requests) def manage_flags(self): - """ Used to interact with charms.reactive-base charms. - """ + """Used to interact with charms.reactive-base charms.""" from charms.reactive import toggle_flag - prefix = 'endpoint.' + self.relation_name - toggle_flag(prefix + '.requests_changed', self.is_changed) + + prefix = "endpoint." + self.relation_name + toggle_flag(prefix + ".requests_changed", self.is_changed) diff --git a/loadbalancer_interface/provider.py b/loadbalancer_interface/provider.py index e08ffee..6c959fa 100644 --- a/loadbalancer_interface/provider.py +++ b/loadbalancer_interface/provider.py @@ -25,8 +25,8 @@ class LBProviderEvents(ObjectEvents): class LBProvider(VersionedInterface): - """ API used to interact with the provider of loadbalancers. - """ + """API used to interact with the provider of loadbalancers.""" + state = StoredState() on = LBProviderEvents() @@ -35,14 +35,15 @@ def __init__(self, charm, relation_name): self.relation_name = relation_name # just call this to enforce that only one app can be related self.model.get_relation(relation_name) - self.state.set_default(response_hashes={}, - was_available=False) - - for event in (charm.on[relation_name].relation_created, - charm.on[relation_name].relation_joined, - charm.on[relation_name].relation_changed, - charm.on[relation_name].relation_departed, - charm.on[relation_name].relation_broken): + self.state.set_default(response_hashes={}, was_available=False) + + for event in ( + charm.on[relation_name].relation_created, + charm.on[relation_name].relation_joined, + charm.on[relation_name].relation_changed, + charm.on[relation_name].relation_departed, + charm.on[relation_name].relation_broken, + ): self.framework.observe(event, self._check_provider) def _check_provider(self, event): @@ -63,19 +64,19 @@ def relation(self): return self.relations[0] if self.relations else None def get_request(self, name): - """ Get or create a Load Balancer Request object. + """Get or create a Load Balancer Request object. May raise a ModelError if unable to create a request. """ if not self.charm.unit.is_leader(): - raise ModelError('Unit is not leader') + raise ModelError("Unit is not leader") if not self.relation: - raise ModelError('Relation not available') + raise ModelError("Relation not available") schema = self._schema(self.relation) local_data = self.relation.data[self.app] remote_data = self.relation.data[self.relation.app] - request_key = 'request_' + name - response_key = 'response_' + name + request_key = "request_" + name + response_key = "response_" + name if request_key in local_data: request_sdata = local_data[request_key] response_sdata = remote_data.get(response_key) @@ -85,7 +86,7 @@ def get_request(self, name): return request def get_response(self, name): - """ Get a specific Load Balancer Response by name. + """Get a specific Load Balancer Response by name. This is equivalent to `get_request(name).response`, except that it will return `None` if the response is not available. @@ -98,85 +99,85 @@ def get_response(self, name): return request.response def send_request(self, request): - """ Send a specific request. + """Send a specific request. May raise a ModelError if unable to send the request. """ if not self.charm.unit.is_leader(): - raise ModelError('Unit is not leader') + raise ModelError("Unit is not leader") if not self.relation: - raise ModelError('Relation not available') + raise ModelError("Relation not available") # The sent_hash is used to tell when the provider's response has been # updated to match our request. We can't used the request hash computed # on the providing side because it may not match due to default values # being filled in on that side (e.g., the backend addresses). request.sent_hash = request.hash - key = 'request_' + request.name + key = "request_" + request.name self.relation.data[self.app][key] = request.dumps() def remove_request(self, name): - """ Remove a specific request. + """Remove a specific request. May raise a ModelError if unable to remove the request. """ if not self.charm.unit.is_leader(): - raise ModelError('Unit is not leader') + raise ModelError("Unit is not leader") if not self.relation: return - key = 'request_' + name + key = "request_" + name self.relation.data[self.app].pop(key, None) self.state.response_hashes.pop(name, None) @property def all_requests(self): - """ A list of all requests which have been made. - """ + """A list of all requests which have been made.""" requests = [] if self.relation: for key in sorted(self.relation.data[self.app].keys()): - if not key.startswith('request_'): + if not key.startswith("request_"): continue - requests.append(self.get_request(key[len('request_'):])) + requests.append(self.get_request(key[len("request_") :])) return requests @property def revoked_responses(self): - """ A list of responses which are no longer available. - """ - return [request.response - for request in self.all_requests - if not request.response - and request.name in self.state.response_hashes] + """A list of responses which are no longer available.""" + return [ + request.response + for request in self.all_requests + if not request.response and request.name in self.state.response_hashes + ] @cached_property def all_responses(self): - """ A list of all responses which are available. - """ - return [request.response - for request in self.all_requests - if request.response] + """A list of all responses which are available.""" + return [request.response for request in self.all_requests if request.response] @cached_property def complete_responses(self): - """ A list of all responses which are up to date with their associated + """A list of all responses which are up to date with their associated request. """ - return [request.response - for request in self.all_requests - if request.response.received_hash == request.sent_hash] + return [ + request.response + for request in self.all_requests + if request.response.received_hash == request.sent_hash + ] @property def new_responses(self): - """ A list of complete responses which have not yet been acknowledged as + """A list of complete responses which have not yet been acknowledged as handled or which have changed. """ acked_responses = self.state.response_hashes - return [response - for response in self.complete_responses - if response.hash != acked_responses.get(response.name)] + return [ + response + for response in self.complete_responses + if response.hash != acked_responses.get(response.name) + ] def ack_response(self, response): - """ Acknowledge that a given response has been handled. + """Acknowledge that a given response has been handled. Can be called on a revoked response as well to remove it from the revoked_responses list. @@ -188,8 +189,9 @@ def ack_response(self, response): if not self.is_changed: try: from charms.reactive import clear_flag - prefix = 'endpoint.' + self.relation_name - clear_flag(prefix + '.responses_changed') + + prefix = "endpoint." + self.relation_name + clear_flag(prefix + ".responses_changed") except ImportError: pass # not being used in a reactive charm @@ -206,9 +208,9 @@ def can_request(self): return self.is_available and self.unit.is_leader() def manage_flags(self): - """ Used to interact with charms.reactive-base charms. - """ + """Used to interact with charms.reactive-base charms.""" from charms.reactive import toggle_flag - prefix = 'endpoint.' + self.relation_name - toggle_flag(prefix + '.available', self.is_available) - toggle_flag(prefix + '.responses_changed', self.is_changed) + + prefix = "endpoint." + self.relation_name + toggle_flag(prefix + ".available", self.is_available) + toggle_flag(prefix + ".responses_changed", self.is_changed) diff --git a/loadbalancer_interface/schemas/__init__.py b/loadbalancer_interface/schemas/__init__.py index 9985edf..ace9f9a 100644 --- a/loadbalancer_interface/schemas/__init__.py +++ b/loadbalancer_interface/schemas/__init__.py @@ -2,12 +2,12 @@ from pathlib import Path versions = {} -for subpath in Path(__file__).parent.glob('*.py'): +for subpath in Path(__file__).parent.glob("*.py"): name = subpath.stem if name == __name__: continue - submod = import_module(__package__ + '.' + name) - if hasattr(submod, 'version'): + submod = import_module(__package__ + "." + name) + if hasattr(submod, "version"): versions[submod.version] = submod max_version = max(versions.keys()) diff --git a/loadbalancer_interface/schemas/base.py b/loadbalancer_interface/schemas/base.py index 6c9f7d9..5a12ed7 100644 --- a/loadbalancer_interface/schemas/base.py +++ b/loadbalancer_interface/schemas/base.py @@ -39,7 +39,7 @@ def dump(self): for field_name, field in self._schema.fields.items(): value = getattr(self, field_name, None) try: - if hasattr(field, '_validated'): + if hasattr(field, "_validated"): # For some reason, some field types do their validation in # a `_validated` method, rather than in the `_validate` # from the base Field class. For those, calling `_validate` @@ -64,6 +64,6 @@ def dumps(self): @property def hash(self): try: - return md5(self.dumps().encode('utf8')).hexdigest() + return md5(self.dumps().encode("utf8")).hexdigest() except ValidationError: return None diff --git a/loadbalancer_interface/schemas/v1.py b/loadbalancer_interface/schemas/v1.py index 8e8c9e2..d9e75c9 100644 --- a/loadbalancer_interface/schemas/v1.py +++ b/loadbalancer_interface/schemas/v1.py @@ -22,10 +22,10 @@ class _Schema(Schema): @validates_schema def _validate(self, data, **kwargs): - if data['success'] and not data['address']: - raise ValidationError('address required on success') - if not data['success'] and not data['message']: - raise ValidationError('message required on failure') + if data["success"] and not data["address"]: + raise ValidationError("address required on success") + if not data["success"] and not data["message"]: + raise ValidationError("message required on failure") def __init__(self, request): super().__init__() @@ -76,11 +76,11 @@ class _Schema(Schema): @classmethod def _from_id(cls, req_id, relations): - """ Return an empty Request with the given ID. + """Return an empty Request with the given ID. This represents an unknown or removed request. """ - name, rel_id = req_id.split(':') + name, rel_id = req_id.split(":") request = cls(name) request._id = req_id for relation in relations: @@ -107,7 +107,7 @@ def id(self): if self._id is None: if self.relation is None: return None - self._id = '{}:{}'.format(self.relation.id, self.name) + self._id = "{}:{}".format(self.relation.id, self.name) return self._id @classmethod @@ -119,8 +119,7 @@ def loads(cls, name, request_sdata, response_sdata=None): return self def add_health_check(self, **kwargs): - """ Create a HealthCheck and add it to the list. - """ + """Create a HealthCheck and add it to the list.""" health_check = HealthCheck()._update(kwargs) self.health_checks.append(health_check) return health_check diff --git a/tests/conftest.py b/tests/conftest.py index 39103e9..97c0ee9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,11 +5,13 @@ # Cached properties are useful at runtime for efficiency, but make unit # testing harder. This simply replaces @cached_property with @property. -mock.patch('cached_property.cached_property', property).start() +mock.patch("cached_property.cached_property", property).start() -if not hasattr(builtins, 'breakpoint'): +if not hasattr(builtins, "breakpoint"): # Shim breakpoint() builtin from PEP-0553 prior to 3.7 def _breakpoint(): import ipdb + ipdb.set_trace(inspect.currentframe().f_back) + builtins.breakpoint = _breakpoint diff --git a/tests/functional/test_interface.py b/tests/functional/test_interface.py index 64bf8d7..537ef38 100644 --- a/tests/functional/test_interface.py +++ b/tests/functional/test_interface.py @@ -18,34 +18,37 @@ def test_interface(): # Helpers def add_peer_unit(harness): units = [harness.charm.unit] + peer_units[harness] - next_unit_num = max(int(unit.name.split('/')[1]) for unit in units) + 1 - new_unit = Unit('/'.join((harness.charm.app.name, str(next_unit_num))), - harness.charm.unit._backend, - harness.charm.unit._cache) + next_unit_num = max(int(unit.name.split("/")[1]) for unit in units) + 1 + new_unit = Unit( + "/".join((harness.charm.app.name, str(next_unit_num))), + harness.charm.unit._backend, + harness.charm.unit._cache, + ) peer_units[harness].append(new_unit) return new_unit def get_rel_data(harness, src): - if hasattr(src, 'name'): + if hasattr(src, "name"): src = src.name return harness.get_relation_data(harness._rid, src) def update_rel_data(harness, data_map): for src, data in data_map.items(): - if hasattr(src, 'name'): + if hasattr(src, "name"): src = src.name harness.update_relation_data(harness._rid, src, data) def transmit_rel_data(src_harness, dst_harness): data = {} - for src in [src_harness.charm.app, - src_harness.charm.unit] + peer_units[src_harness]: + for src in [src_harness.charm.app, src_harness.charm.unit] + peer_units[ + src_harness + ]: src_data = get_rel_data(src_harness, src) dst_data = get_rel_data(dst_harness, src) # Removed keys have to be explicitly set to an empty string for the # harness to remove them. (TODO: file upstream bug) for removed in dst_data.keys() - src_data.keys(): - src_data[removed] = '' + src_data[removed] = "" data[src] = src_data update_rel_data(dst_harness, data) @@ -57,8 +60,8 @@ def transmit_rel_data(src_harness, dst_harness): c_unit1 = add_peer_unit(consumer) # Setup initial relation with only Juju-provided automatic data. - provider._rid = provider.add_relation('lb-consumers', c_charm.meta.name) - consumer._rid = consumer.add_relation('lb-provider', p_charm.meta.name) + provider._rid = provider.add_relation("lb-consumers", c_charm.meta.name) + consumer._rid = consumer.add_relation("lb-provider", p_charm.meta.name) # NB: The first unit added to the relation determines the app of the # relation, so it's critical to add a remote unit before any local units. provider.add_relation_unit(provider._rid, c_unit0.name) @@ -67,18 +70,21 @@ def transmit_rel_data(src_harness, dst_harness): consumer.add_relation_unit(consumer._rid, p_charm.unit.name) consumer.add_relation_unit(consumer._rid, c_unit0.name) consumer.add_relation_unit(consumer._rid, c_unit1.name) - update_rel_data(consumer, { - c_unit1: {'ingress-address': '192.168.0.3'}, - c_unit0: {'ingress-address': '192.168.0.5'}, - }) + update_rel_data( + consumer, + { + c_unit1: {"ingress-address": "192.168.0.3"}, + c_unit0: {"ingress-address": "192.168.0.5"}, + }, + ) - foo_id = '{}:foo'.format(provider._rid) - bar_id = '{}:bar'.format(provider._rid) + foo_id = "{}:foo".format(provider._rid) + bar_id = "{}:bar".format(provider._rid) # Confirm that only leaders set the version. assert not get_rel_data(provider, p_app) provider.set_leader(True) - assert get_rel_data(provider, p_app) == {'version': '1'} + assert get_rel_data(provider, p_app) == {"version": "1"} assert not c_charm.lb_provider.is_available # waiting on remote version assert not c_charm.lb_provider.can_request # waiting on remote version @@ -91,60 +97,65 @@ def transmit_rel_data(src_harness, dst_harness): # allows sending requests. consumer.set_leader(True) assert c_charm.lb_provider.can_request - assert get_rel_data(consumer, c_app) == {'version': '1'} + assert get_rel_data(consumer, c_app) == {"version": "1"} transmit_rel_data(consumer, provider) # Test creating and sending a request. - c_charm.request_lb('foo') + c_charm.request_lb("foo") transmit_rel_data(consumer, provider) assert foo_id in p_charm.lb_consumers.state.known_requests - assert p_charm.lb_consumers.all_requests[0].backends == ['192.168.0.5', - '192.168.0.3'] - assert p_charm.changes == {'foo': 1} + assert p_charm.lb_consumers.all_requests[0].backends == [ + "192.168.0.5", + "192.168.0.3", + ] + assert p_charm.changes == {"foo": 1} # Test receiving the response assert not c_charm.active_lbs assert not c_charm.failed_lbs transmit_rel_data(provider, consumer) - assert c_charm.changes == {'foo': 1} - assert c_charm.active_lbs == {'foo'} + assert c_charm.changes == {"foo": 1} + assert c_charm.active_lbs == {"foo"} assert not c_charm.failed_lbs - assert c_charm.lb_provider.get_response('foo').address == 'lb-foo' + assert c_charm.lb_provider.get_response("foo").address == "lb-foo" # Test default updates being tracked - update_rel_data(consumer, { - c_unit1: {'ingress-address': '192.168.0.4'}, - c_unit0: {'ingress-address': '192.168.0.6'}, - }) + update_rel_data( + consumer, + { + c_unit1: {"ingress-address": "192.168.0.4"}, + c_unit0: {"ingress-address": "192.168.0.6"}, + }, + ) transmit_rel_data(consumer, provider) transmit_rel_data(provider, consumer) - assert p_charm.changes == {'foo': 3} + assert p_charm.changes == {"foo": 3} # Note: Since the request didn't change, the requires side doesn't see # a change in the response. - assert c_charm.changes == {'foo': 1} - assert c_charm.active_lbs == {'foo'} + assert c_charm.changes == {"foo": 1} + assert c_charm.active_lbs == {"foo"} assert not c_charm.failed_lbs # Test updating the request and getting an updated response. - c_charm.request_lb('foo', ['192.168.0.5']) + c_charm.request_lb("foo", ["192.168.0.5"]) transmit_rel_data(consumer, provider) transmit_rel_data(provider, consumer) - assert p_charm.lb_consumers.all_requests[0].backends == ['192.168.0.5'] - assert p_charm.changes == {'foo': 4} - assert c_charm.changes == {'foo': 2} - assert c_charm.active_lbs == {'foo'} + assert p_charm.lb_consumers.all_requests[0].backends == ["192.168.0.5"] + assert p_charm.changes == {"foo": 4} + assert c_charm.changes == {"foo": 2} + assert c_charm.active_lbs == {"foo"} assert not c_charm.failed_lbs # Test sending a second request - c_charm.request_lb('bar') + c_charm.request_lb("bar") transmit_rel_data(consumer, provider) transmit_rel_data(provider, consumer) assert bar_id in p_charm.lb_consumers.state.known_requests - assert c_charm.active_lbs == {'foo'} - assert c_charm.failed_lbs == {'bar'} + assert c_charm.active_lbs == {"foo"} + assert c_charm.failed_lbs == {"bar"} # Test request removal - c_charm.lb_provider.remove_request('bar') + c_charm.lb_provider.remove_request("bar") transmit_rel_data(consumer, provider) assert foo_id in p_charm.lb_consumers.state.known_requests assert bar_id not in p_charm.lb_consumers.state.known_requests @@ -167,10 +178,9 @@ class ProviderCharm(CharmBase): def __init__(self, *args): super().__init__(*args) - self.lb_consumers = LBConsumers(self, 'lb-consumers') + self.lb_consumers = LBConsumers(self, "lb-consumers") - self.framework.observe(self.lb_consumers.on.requests_changed, - self._update_lbs) + self.framework.observe(self.lb_consumers.on.requests_changed, self._update_lbs) self.changes = {} @@ -178,12 +188,12 @@ def _update_lbs(self, event): for request in self.lb_consumers.new_requests: self.changes.setdefault(request.name, 0) self.changes[request.name] += 1 - if request.name == 'foo': + if request.name == "foo": request.response.success = True - request.response.address = 'lb-' + request.name + request.response.address = "lb-" + request.name else: request.response.success = False - request.response.message = 'No reason' + request.response.message = "No reason" self.lb_consumers.send_response(request) for request in self.lb_consumers.removed_requests: self.lb_consumers.revoke_response(request) @@ -200,10 +210,9 @@ class ConsumerCharm(CharmBase): def __init__(self, *args): super().__init__(*args) self._to_break = False - self.lb_provider = LBProvider(self, 'lb-provider') + self.lb_provider = LBProvider(self, "lb-provider") - self.framework.observe(self.lb_provider.on.responses_changed, - self._update_lbs) + self.framework.observe(self.lb_provider.on.responses_changed, self._update_lbs) self.changes = {} self.active_lbs = set() @@ -211,7 +220,7 @@ def __init__(self, *args): def request_lb(self, name, backends=None): request = self.lb_provider.get_request(name) - request.traffic_type = 'https' + request.traffic_type = "https" request.backend_ports = [443] if backends is not None: request.backends = backends diff --git a/tests/unit/test_schema_v1.py b/tests/unit/test_schema_v1.py index 9e6ea23..9961e81 100644 --- a/tests/unit/test_schema_v1.py +++ b/tests/unit/test_schema_v1.py @@ -26,85 +26,83 @@ def is_not_changed(key, new): def test_request(): with pytest.raises(TypeError): - Request(foo='foo') + Request(foo="foo") with pytest.raises(ValidationError): - Request('name')._update(traffic_type='https', - backend_ports=['none']) + Request("name")._update(traffic_type="https", backend_ports=["none"]) with pytest.raises(ValidationError): - Request('name')._update(traffic_type='https', - backend_ports=[443], - foo='bar') + Request("name")._update(traffic_type="https", backend_ports=[443], foo="bar") - req = Request('name') - req.traffic_type = 'https' + req = Request("name") + req.traffic_type = "https" req.backend_ports = [443] assert req.version == 1 assert req.health_checks == [] assert req.dump() assert req.id is None - req.relation = Mock(id='0') - assert req.id == '0:name' + req.relation = Mock(id="0") + assert req.id == "0:name" - hc = HealthCheck()._update(traffic_type='https', port=443) + hc = HealthCheck()._update(traffic_type="https", port=443) req.health_checks.append(hc) - assert is_set_and_changed('req.hash', req.hash) + assert is_set_and_changed("req.hash", req.hash) req.sent_hash = req.hash - assert is_set_and_changed('req.hash', req.hash) + assert is_set_and_changed("req.hash", req.hash) hc.port = 6443 - assert is_set_and_changed('req.hash', req.hash) - req.repsonse = 'foo' - assert is_not_changed('req.hash', req.hash) + assert is_set_and_changed("req.hash", req.hash) + req.repsonse = "foo" + assert is_not_changed("req.hash", req.hash) req.traffic_type = None with pytest.raises(ValidationError): req.dump() assert req.hash is None - req.traffic_type = 'https' - - req2 = Request.loads('name', req.dumps(), '{' - ' "success": true,' - ' "address": "foo",' - ' "received_hash": "%s"' - '}' % req.sent_hash) + req.traffic_type = "https" + + req2 = Request.loads( + "name", + req.dumps(), + "{" + ' "success": true,' + ' "address": "foo",' + ' "received_hash": "%s"' + "}" % req.sent_hash, + ) assert req2.hash == req.hash assert req2.response.success - assert req2.response.address == 'foo' + assert req2.response.address == "foo" assert req2.response.received_hash == req.sent_hash def test_response(): - request = Mock(name='request') - request.name = 'name' + request = Mock(name="request") + request.name = "name" with pytest.raises(TypeError): Response() with pytest.raises(ValidationError): - Response(request)._update(success=True, - address=None, - received_hash=None) + Response(request)._update(success=True, address=None, received_hash=None) with pytest.raises(ValidationError): - Response(request)._update(success=True, - address='https://my-lb.aws.com/', - received_hash='', - foo='bar') + Response(request)._update( + success=True, address="https://my-lb.aws.com/", received_hash="", foo="bar" + ) with pytest.raises(ValidationError): - Response(request)._update(success=False, - address='https://my-lb.aws.com/', - received_hash='') - - resp = Response(request)._update(success=True, - address='https://my-lb.aws.com/', - received_hash='') - assert resp.name == 'name' - assert is_set_and_changed('resp.hash', resp.hash) - resp.foo = 'bar' - assert is_not_changed('resp.hash', resp.hash) + Response(request)._update( + success=False, address="https://my-lb.aws.com/", received_hash="" + ) + + resp = Response(request)._update( + success=True, address="https://my-lb.aws.com/", received_hash="" + ) + assert resp.name == "name" + assert is_set_and_changed("resp.hash", resp.hash) + resp.foo = "bar" + assert is_not_changed("resp.hash", resp.hash) resp.success = False with pytest.raises(ValidationError): resp.dump() assert resp.hash is None - resp.message = 'foo' - assert is_set_and_changed('resp.hash', resp.hash) + resp.message = "foo" + assert is_set_and_changed("resp.hash", resp.hash) resp2 = Response(request)._update(resp.dump()) assert resp2.hash == resp.hash @@ -112,37 +110,33 @@ def test_response(): def test_health_check(): with pytest.raises(TypeError): - HealthCheck('foo') + HealthCheck("foo") with pytest.raises(TypeError): - HealthCheck(traffic_type='foo') + HealthCheck(traffic_type="foo") with pytest.raises(ValidationError): HealthCheck().dump() with pytest.raises(ValidationError): hc = HealthCheck() - hc.traffic_type = 'https' - hc.port = 'none' + hc.traffic_type = "https" + hc.port = "none" hc.dump() with pytest.raises(ValidationError): - HealthCheck()._update(traffic_type='https', - port=443, - foo='bar') + HealthCheck()._update(traffic_type="https", port=443, foo="bar") - hc = HealthCheck()._update(traffic_type='https', port=443) + hc = HealthCheck()._update(traffic_type="https", port=443) assert hc.version == 1 - assert hc.traffic_type == 'https' + assert hc.traffic_type == "https" assert hc.port == 443 assert hc.path is None assert hc.interval == 30 assert hc.retries == 3 - hc = HealthCheck()._update(traffic_type='http', - port=80, - path='/foo', - interval=60, - retries=5) - assert hc.traffic_type == 'http' + hc = HealthCheck()._update( + traffic_type="http", port=80, path="/foo", interval=60, retries=5 + ) + assert hc.traffic_type == "http" assert hc.port == 80 - assert hc.path == '/foo' + assert hc.path == "/foo" assert hc.interval == 60 assert hc.retries == 5 - assert hc.hash == HealthCheckField()._deserialize(hc.dump(), '', '').hash + assert hc.hash == HealthCheckField()._deserialize(hc.dump(), "", "").hash diff --git a/tox.ini b/tox.ini index fa454f1..9181ba4 100644 --- a/tox.ini +++ b/tox.ini @@ -10,7 +10,10 @@ setenv = [testenv:lint] deps = flake8 -commands = flake8 --exclude .* + black +commands = + flake8 --exclude .* + black --check loadbalancer_interface tests [testenv:unit] deps = @@ -23,3 +26,13 @@ deps = pytest ipdb commands = pytest --tb native -svv {posargs:tests/functional} + +[testenv:reformat] +commands = + black loadbalancer_interface tests +deps = + black + +[flake8] +max-line-length: 88 +ignore: E203