Fix cross-model dashboards #29
Conversation
…na-operator into fix/cross-model-relations
| @@ -1,3 +1,9 @@ | |||
| /venv | |||
| *.py[cod] | |||
| *.charm | |||
| .flake8 | |||
| CONTRIBUTING.md | ||
| INTEGRATING.md | ||
| RELEASE.md |
There was a problem hiding this comment.
File comment:
iiuc, .jujuignore is only needed to blacklist specific things that got glob-whitelisted by charmcraft.yaml.
In this particular case it seems that this file is not needed at all.
| rlist = [] # type: List[Any] | ||
| for i in obj: | ||
| rlist.append(type_convert_stored(i)) | ||
| return rlist |
There was a problem hiding this comment.
| rlist = [] # type: List[Any] | |
| for i in obj: | |
| rlist.append(type_convert_stored(i)) | |
| return rlist | |
| return list(map(type_convert_stored, obj)) |
| @@ -89,7 +118,8 @@ def __init__( | |||
| self, | |||
| charm: CharmBase, | |||
| name: str, | |||
| event_relation: str = "monitoring", | |||
| event_relation: Optional[str] = "monitoring", | |||
There was a problem hiding this comment.
Based on the code in this PR's diff it seems that None is not a valid value. Drop the Optional?
| event_relation: Optional[str] = "monitoring", | |
| event_relation = "monitoring", |
| [ | ||
| relation.units > 0 | ||
| for relation in self.charm.model.relations[self._stored.event_relation] | ||
| ] |
There was a problem hiding this comment.
Square brackets are redundant here
| [ | |
| relation.units > 0 | |
| for relation in self.charm.model.relations[self._stored.event_relation] | |
| ] | |
| relation.units > 0 | |
| for relation in self.charm.model.relations[self._stored.event_relation] |
| @@ -325,6 +399,7 @@ def __init__(self, charm: CharmBase, name: str) -> None: | |||
| super().__init__(charm, name) | |||
| self.charm = charm | |||
| self.name = name | |||
| self.source_relation = "grafana-source" | |||
There was a problem hiding this comment.
This is invariant - consider moving to class variable.
| # Figure out our Prometheus relation and template the query | ||
|
|
||
| try: | ||
| prom_rel = self.charm.model.relations[self.source_relation][0] |
| @@ -398,32 +493,40 @@ def _validate_dashboard_data(self, data: Dict, rel: Relation) -> None: | |||
| if not grafana_datasource: | |||
| return | |||
|
|
|||
| # Import at runtime so we don't get client dependencies | |||
There was a problem hiding this comment.
I don't understand this. What do you mean by "client dependency"? Why here is "at runtime" compared to top of file?
| event: a :class:`UpgradeCharmEvent` to signal the upgrade | ||
| """ | ||
| self._configure(event) | ||
| self._on_dashboards_changed(event) |
There was a problem hiding this comment.
Would it make sense to move this into the provider lib, similar to what you have with the consumer?
Unless _configure must happen before..?
|
@sed-i I did not see your comments before merging this, I'll address them in a PR I am working on to streamline the constructors |
No description provided.