Code review comment for ~jk0ne/charm-k8s-discourse/+git/charm-k8s-discourse:master

Some other comments from looking at the code outside of this MP.

In src/charm.py can we alpha-sort `from ops.model import MaintenanceStatus, BlockedStatus, ActiveStatus`?

Per a comment we got on the jenkins-agent charm recently, "Please use "store", not "state", for StoredState (with undercase if you want to make it private)." You'll obviously need to update a all the references to self.state as appropriate too.

In check_for_config_problems you have "if len(missing_fields):" but this could just be "if missing_fields:".

In check_config_is_valid you have "if len(errors) > 0:" but this could just be "if errors:".

Please alpha sort imports in the unit tests.

I'm not sure why you have 'dirname = os.path.dirname(__file__)' as a global variable in unit tests. Seems like you could just remove that and replace line 48 with:

self.configs = load_configs(os.path.join(os.path.dirname(__file__), 'fixtures'))

I don't think we need to import (or use) pprint in the unit tests any more? Looks like it was just for debugging of the load_configs function.

And lastly, just a suggestion, feel free to ignore if you like. In load_configs you have:

name = os.path.splitext(os.path.basename(filename))
configs[name[0]] = yaml.full_load(file)

This could be (which seems a little easier to follow to me, but YMMV):

name, _ = os.path.splitext(os.path.basename(filename))
configs[name] = yaml.full_load(file)