sts-sponsors team mailing list archive

[Adam Collard <mp+434563@xxxxxxxxxxxxxxxxxx>]

Review: Approve

nit picks

Diff comments:

> diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
> index 3aba41b..e07cfbe 100644
> --- a/src/maasserver/models/tests/test_node.py
> +++ b/src/maasserver/models/tests/test_node.py
> @@ -4823,9 +4823,10 @@ class TestNode(MAASServerTestCase):
>          self.disable_node_query()
>          node = factory.make_Node(status=NODE_STATUS.DEPLOYING)
>          node.end_deployment()
> -        event = Event.objects.get(node=node)
> +        events = Event.objects.filter(node=node)
>          self.assertEqual(NODE_STATUS.DEPLOYED, reload_object(node).status)
> -        self.assertEqual(event.type.name, EVENT_TYPES.DEPLOYED)
> +        self.assertEqual(events[0].type.name, EVENT_TYPES.IMAGE_DEPLOYED)
> +        self.assertEqual(events[1].type.name, EVENT_TYPES.DEPLOYED)
>  

i'm not sure we can rely on the order of these two events coming back from the .filter

how about creating a set() like event_types = {event.type.name for event in events} and checking that for equality against {EVENT_TYPES.IMAGE_DEPLOYED, EVENT_TYPES.DEPLOYED}

>      def test_end_deployment_sets_first_last_sync_value(self):
>          self.disable_node_query()
> diff --git a/src/maasserver/websockets/handlers/bootresource.py b/src/maasserver/websockets/handlers/bootresource.py
> index 908af1e..4191c3a 100644
> --- a/src/maasserver/websockets/handlers/bootresource.py
> +++ b/src/maasserver/websockets/handlers/bootresource.py
> @@ -431,6 +436,19 @@ class BootResourceHandler(Handler):
>              for resource in resources
>          )
>  
> +    def get_last_deployed_for_resources(
> +        self, resources: list[BootResource]
> +    ) -> datetime:

this is an inaccurate type annotation - it should be Optional[datetime] since it can return None

> +        """Return the most recent deploy time for all resources."""
> +        last_deployed = None
> +        for resource in resources:
> +            this_deploy = resource.get_last_deploy()
> +            if this_deploy is not None:
> +                last_deployed = self.pick_latest_datetime(
> +                    last_deployed, this_deploy
> +                )
> +        return last_deployed
> +
>      def get_progress_for_resources(self, resources):
>          """Return the overall progress for all resources."""
>          size = 0


-- 
https://code.launchpad.net/~lloydwaltersj/maas/+git/maas/+merge/434563
Your team MAAS Committers is subscribed to branch maas:master.