feat: narrow the type for 'app' and 'unit' in relation events

ops/charm.py

@@ -369,11 +369,12 @@ class RelationEvent(HookEvent):
     """
 
     relation: 'Relation'
-    """The relation involved in this event."""
+    """The relation involved in this event.
+    """
 
-    # TODO(benhoyt): I *think* app should never be None, but confirm and update type
-    app: Optional[model.Application]
-    """The remote application that has triggered this event."""
+    app: model.Application
+    """The remote application that has triggered this event.
+    """
 
     unit: Optional[model.Unit]
     """The remote unit that has triggered this event.
@@ -392,7 +393,12 @@ def __init__(self, handle: 'Handle', relation: 'Relation',
                 f'cannot create RelationEvent with application {app} and unit {unit}')
 
         self.relation = relation
-        self.app = app
+        if app is None:
+            logger.warning("'app' expected but not received.")
+            # Do an explicit assignment here so that we can contain the type: ignore.
+            self.app = None  # type: ignore
+        else:
+            self.app = app
         self.unit = unit
 
     def snapshot(self) -> Dict[str, Any]:
@@ -427,7 +433,8 @@ def restore(self, snapshot: Dict[str, Any]):
         if app_name:
             self.app = self.framework.model.get_app(app_name)
         else:
-            self.app = None
+            logger.warning("'app_name' expected in snapshot but not found.")
+            self.app = None  # type: ignore
 
         unit_name = snapshot.get('unit_name')
         if unit_name:
@@ -443,6 +450,9 @@ class RelationCreatedEvent(RelationEvent):
     can occur before units for those applications have started. All existing
     relations should be established before start.
     """
+    unit: None
+    """Always ``None``.
+    """
 
 
 class RelationJoinedEvent(RelationEvent):
@@ -456,6 +466,9 @@ class RelationJoinedEvent(RelationEvent):
     remote ``private-address`` setting, which is always available when
     the relation is created and is by convention not deleted.
     """
+    unit: model.Unit
+    """The remote unit that has triggered this event.
+    """
 
 
 class RelationChangedEvent(RelationEvent):
@@ -496,6 +509,9 @@ class RelationDepartedEvent(RelationEvent):
     Once all callback methods bound to this event have been run for such a
     relation, the unit agent will fire the :class:`RelationBrokenEvent`.
     """
+    unit: model.Unit
+    """The remote unit that has triggered this event.
+    """
 
     def __init__(self, handle: 'Handle', relation: 'Relation',
                  app: Optional[model.Application] = None,
@@ -551,6 +567,9 @@ class RelationBrokenEvent(RelationEvent):
     bound to this event is being executed, it is guaranteed that no remote units
     are currently known locally.
     """
+    unit: None
+    """Always ``None``.
+    """
 
 
 class StorageEvent(HookEvent):

test/test_charm.py

@@ -190,19 +190,22 @@ def on_any_relation(self, event: ops.RelationEvent):
         self.assertIn('pro_2_relation_broken', repr(charm.on))
 
         rel = charm.framework.model.get_relation('req1', 1)
+        app = charm.framework.model.get_app('remote')
         unit = charm.framework.model.get_unit('remote/0')
-        charm.on['req1'].relation_joined.emit(rel, unit)
-        charm.on['req1'].relation_changed.emit(rel, unit)
-        charm.on['req-2'].relation_changed.emit(rel, unit)
-        charm.on['pro1'].relation_departed.emit(rel, unit)
-        charm.on['pro-2'].relation_departed.emit(rel, unit)
-        charm.on['peer1'].relation_broken.emit(rel)
-        charm.on['peer-2'].relation_broken.emit(rel)
+        charm.on['req1'].relation_joined.emit(rel, app, unit)
+        charm.on['req1'].relation_changed.emit(rel, app, unit)
+        charm.on['req1'].relation_changed.emit(rel, app)
+        charm.on['req-2'].relation_changed.emit(rel, app, unit)
+        charm.on['pro1'].relation_departed.emit(rel, app, unit)
+        charm.on['pro-2'].relation_departed.emit(rel, app, unit)
+        charm.on['peer1'].relation_broken.emit(rel, app)
+        charm.on['peer-2'].relation_broken.emit(rel, app)
 
         self.assertEqual(charm.seen, [
             'RelationJoinedEvent',
             'RelationChangedEvent',
             'RelationChangedEvent',
+            'RelationChangedEvent',
             'RelationDepartedEvent',
             'RelationDepartedEvent',
             'RelationBrokenEvent',

test/test_main.py

@@ -408,10 +408,16 @@ def _simulate_event(self, event_spec: EventSpec):
             if remote_app is not None:
                 env['JUJU_REMOTE_APP'] = remote_app
 
-            remote_unit = event_spec.remote_unit
-            if remote_unit is None:
-                remote_unit = ''
-            env['JUJU_REMOTE_UNIT'] = remote_unit
+            if (remote_app is None
+                or issubclass(event_spec.event_type, ops.RelationJoinedEvent)
+                or issubclass(event_spec.event_type, ops.RelationChangedEvent)
+                    or issubclass(event_spec.event_type, ops.RelationDepartedEvent)):
+                remote_unit = event_spec.remote_unit
+                if remote_unit is None:
+                    remote_unit = ''
+                env['JUJU_REMOTE_UNIT'] = remote_unit
+            else:
+                env['JUJU_REMOTE_UNIT'] = ''
 
             departing_unit_name = event_spec.departing_unit_name
             if departing_unit_name is None: