Skip to content

Commit 3fddbf4

Browse files
Simplify reconcile
Avoid passing refresh object since we know reconcile will be called later (no need to explicitly call it & have it run twice)
1 parent fd4d12a commit 3fddbf4

File tree

2 files changed

+34
-31
lines changed

2 files changed

+34
-31
lines changed

src/abstract_charm.py

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ def _cos_exporter_config(self, event) -> typing.Optional[relations.cos.ExporterC
173173
if cos_relation_exists:
174174
return self._cos_relation.exporter_user_config
175175

176-
def get_workload(self, *, event, refresh: charm_refresh.Common):
176+
def get_workload(self, *, event, refresh: charm_refresh.Common = None):
177177
"""MySQL Router workload
178178
179179
Pass `refresh` if `self.refresh` is not set
@@ -214,10 +214,10 @@ def _prioritize_statuses(statuses: typing.List[ops.StatusBase]) -> ops.StatusBas
214214
return status
215215
return ops.ActiveStatus()
216216

217-
def _determine_app_status(self, *, event, refresh: charm_refresh.Common) -> ops.StatusBase:
217+
def _determine_app_status(self, *, event) -> ops.StatusBase:
218218
"""Report app status."""
219-
if refresh.app_status_higher_priority:
220-
return refresh.app_status_higher_priority
219+
if self.refresh.app_status_higher_priority:
220+
return self.refresh.app_status_higher_priority
221221
statuses = []
222222
if self._status:
223223
statuses.append(self._status)
@@ -226,32 +226,32 @@ def _determine_app_status(self, *, event, refresh: charm_refresh.Common) -> ops.
226226
statuses.append(status)
227227
return self._prioritize_statuses(statuses)
228228

229-
def _determine_unit_status(self, *, event, refresh: charm_refresh.Common) -> ops.StatusBase:
229+
def _determine_unit_status(self, *, event) -> ops.StatusBase:
230230
"""Report unit status."""
231-
if refresh.unit_status_higher_priority:
232-
return refresh.unit_status_higher_priority
231+
if self.refresh.unit_status_higher_priority:
232+
return self.refresh.unit_status_higher_priority
233233
statuses = []
234-
workload_ = self.get_workload(event=event, refresh=refresh)
234+
workload_ = self.get_workload(event=event)
235235
if status := workload_.status:
236236
statuses.append(status)
237237
# only in machine charms
238238
if self._ha_cluster:
239239
if status := self._ha_cluster.get_unit_juju_status():
240240
statuses.append(status)
241-
refresh_lower_priority = refresh.unit_status_lower_priority(
241+
refresh_lower_priority = self.refresh.unit_status_lower_priority(
242242
workload_is_running=isinstance(workload_, workload.RunningWorkload)
243243
)
244244
if (not statuses or statuses == [ops.WaitingStatus()]) and refresh_lower_priority:
245245
return refresh_lower_priority
246246
return self._prioritize_statuses(statuses)
247247

248-
def set_status(self, *, event, refresh: charm_refresh.Common, app=True, unit=True) -> None:
248+
def set_status(self, *, event, app=True, unit=True) -> None:
249249
"""Set charm status."""
250250
if app and self._unit_lifecycle.authorized_leader:
251-
self.app.status = self._determine_app_status(event=event, refresh=refresh)
251+
self.app.status = self._determine_app_status(event=event)
252252
logger.debug(f"Set app status to {self.app.status}")
253253
if unit:
254-
self.unit.status = self._determine_unit_status(event=event, refresh=refresh)
254+
self.unit.status = self._determine_unit_status(event=event)
255255
logger.debug(f"Set unit status to {self.unit.status}")
256256

257257
@abc.abstractmethod
@@ -283,28 +283,26 @@ def _update_endpoints(self) -> None:
283283
# Handlers
284284
# =======================
285285

286-
def reconcile(self, event=None, *, refresh: charm_refresh.Common = None) -> None: # noqa: C901
286+
def reconcile(self, event=None) -> None: # noqa: C901
287287
"""Handle most events."""
288-
if refresh is None:
289-
refresh = self.refresh
290-
workload_ = self.get_workload(event=event, refresh=refresh)
288+
workload_ = self.get_workload(event=event)
291289
logger.debug(
292290
"State of reconcile "
293291
f"{self._unit_lifecycle.authorized_leader=}, "
294292
f"{isinstance(workload_, workload.RunningWorkload)=}, "
295293
f"{workload_.container_ready=}, "
296-
f"{refresh.workload_allowed_to_start=}, "
294+
f"{self.refresh.workload_allowed_to_start=}, "
297295
f"{self._database_requires.is_relation_breaking(event)=}, "
298296
f"{self._database_requires.does_relation_exist()=}, "
299-
f"{refresh.in_progress=}, "
297+
f"{self.refresh.in_progress=}, "
300298
f"{self._cos_relation.is_relation_breaking(event)=}"
301299
)
302-
if isinstance(refresh, charm_refresh.Machines):
300+
if isinstance(self.refresh, charm_refresh.Machines):
303301
workload_.install(
304302
unit=self.unit,
305303
model_uuid=self.model.uuid,
306-
snap_revision=refresh.pinned_snap_revision,
307-
refresh=refresh,
304+
snap_revision=self.refresh.pinned_snap_revision,
305+
refresh=self.refresh,
308306
)
309307
self.unit.set_workload_version(workload_.version)
310308

@@ -315,13 +313,13 @@ def reconcile(self, event=None, *, refresh: charm_refresh.Common = None) -> None
315313
try:
316314
if self._unit_lifecycle.authorized_leader:
317315
if self._database_requires.is_relation_breaking(event):
318-
if refresh.in_progress:
316+
if self.refresh.in_progress:
319317
logger.warning(
320318
"Modifying relations during an upgrade is not supported. The charm may be in a broken, unrecoverable state. Re-deploy the charm"
321319
)
322320
self._database_provides.delete_all_databags()
323321
elif (
324-
not refresh.in_progress
322+
not self.refresh.in_progress
325323
and isinstance(workload_, workload.RunningWorkload)
326324
and workload_.container_ready
327325
):
@@ -346,13 +344,17 @@ def reconcile(self, event=None, *, refresh: charm_refresh.Common = None) -> None
346344
certificate=self._tls_certificate,
347345
certificate_authority=self._tls_certificate_authority,
348346
)
349-
if not refresh.in_progress and isinstance(workload_, workload.RunningWorkload):
347+
if not self.refresh.in_progress and isinstance(
348+
workload_, workload.RunningWorkload
349+
):
350350
self._reconcile_ports(event=event)
351351

352352
logger.debug(f"{workload_.status=}")
353353
if not workload_.status:
354-
refresh.next_unit_allowed_to_refresh = True
355-
elif refresh.workload_allowed_to_start and workload_.status == ops.WaitingStatus():
354+
self.refresh.next_unit_allowed_to_refresh = True
355+
elif (
356+
self.refresh.workload_allowed_to_start and workload_.status == ops.WaitingStatus()
357+
):
356358
# During scale up, this code should not be reached before the first
357359
# relation-created event is received on this unit since otherwise
358360
# `charm_refresh.PeerRelationNotReady` would be raised
@@ -361,10 +363,10 @@ def reconcile(self, event=None, *, refresh: charm_refresh.Common = None) -> None
361363
pass
362364
else:
363365
# Waiting for database requires relation; refresh can continue
364-
refresh.next_unit_allowed_to_refresh = True
365-
self.set_status(event=event, refresh=refresh)
366+
self.refresh.next_unit_allowed_to_refresh = True
367+
self.set_status(event=event)
366368
except server_exceptions.Error as e:
367369
# If not for `unit=False`, another `server_exceptions.Error` could be thrown here
368-
self.set_status(event=event, refresh=refresh, unit=False)
370+
self.set_status(event=event, unit=False)
369371
self.unit.status = e.status
370372
logger.debug(f"Set unit status to {self.unit.status}")

src/charm.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,9 @@ def refresh_snap(
5353
tls=self._charm._tls_certificate_saved,
5454
exporter_config=self._charm._cos_exporter_config(event=None),
5555
)
56-
# Set `refresh.next_unit_allowed_to_refresh = True`
57-
self._charm.reconcile(refresh=refresh)
56+
# `reconcile()` will run on every event, which will set
57+
# `refresh.next_unit_allowed_to_refresh = True`
58+
# (This method will run in the charm's __init__, before `reconcile()` is called by ops)
5859

5960

6061
@trace_charm(

0 commit comments

Comments
 (0)