Skip to content

Commit

Permalink
lava_dispatcher: Make Pipeline job attribute always be Job
Browse files Browse the repository at this point in the history
Right now the that attribute can be None but only when running in
unit tests. Instead of having special behavior for unit tests
change the unit tests to properly initialize the Pipeline objects.

Changes to classes:

Pipeline __init__
  First argument is now `job` and is required to be job.
  `job` is assigned to the attribute.

Job __init__
  `device` and `timeout` are now required arguments.
  They are assigned to attributes.

To properly initialize the Job objects inside unit tests a new
method was added to the `LavaDispatcherTestCase` called
`create_simple_job`.

The eventual goal is to make dispatcher pass the `mypy --strict`
type checking. Unfortunately the pytest does not support static
typing so all the unit tests that had to be changed were converted
to unittest style tests.

See pytest this developer comment: pytest-dev/pytest#5981 (comment)
  • Loading branch information
Igor Ponomarev committed Jan 25, 2024
1 parent 8a8a79f commit c53746d
Show file tree
Hide file tree
Showing 25 changed files with 2,268 additions and 2,001 deletions.
17 changes: 9 additions & 8 deletions lava_dispatcher/action.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import traceback
import warnings
from functools import reduce
from typing import TYPE_CHECKING

import pexpect

Expand All @@ -29,6 +30,9 @@
from lava_common.timeout import Timeout
from lava_dispatcher.utils.strings import seconds_to_str

if TYPE_CHECKING:
from .job import Job


class InternalObject:
"""
Expand Down Expand Up @@ -75,7 +79,7 @@ class Pipeline:
of the per-action log handler.
"""

def __init__(self, parent=None, job=None, parameters=None):
def __init__(self, job: Job, parent=None, parameters=None):
self.actions = []
self.parent = None
self.parameters = {} if parameters is None else parameters
Expand Down Expand Up @@ -120,13 +124,10 @@ def add_action(self, action, parameters=None):

# Compute the timeout
global_timeouts = []
# FIXME: Only needed for the auto-tests
if self.job is not None:
if self.job.device is not None:
# First, the device level overrides
global_timeouts.append(self.job.device.get("timeouts", {}))
# Then job level overrides
global_timeouts.append(self.job.parameters.get("timeouts", {}))
# First, the device level overrides
global_timeouts.append(self.job.device.get("timeouts", {}))
# Then job level overrides
global_timeouts.append(self.job.parameters.get("timeouts", {}))

# Set the timeout. The order is:
# 1. global action timeout
Expand Down
23 changes: 20 additions & 3 deletions lava_dispatcher/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# Author: Neil Williams <[email protected]>
#
# SPDX-License-Identifier: GPL-2.0-or-later
from __future__ import annotations

import datetime
import errno
Expand All @@ -11,6 +12,7 @@
import tempfile
import time
import traceback
from typing import TYPE_CHECKING

import pytz

Expand All @@ -23,6 +25,14 @@
MultinodeProtocol,
)

if TYPE_CHECKING:
from logging import Logger
from typing import Any

from lava_common.timeout import Timeout

from .device import Device


class Job:
"""
Expand All @@ -40,17 +50,24 @@ class Job:
device for this job - one job, one device.
"""

def __init__(self, job_id, parameters, logger):
def __init__(
self,
job_id: int,
parameters: dict[str, Any],
logger: Logger,
device: Device,
timeout: Timeout,
):
self.job_id = job_id
self.logger = logger
self.device = None
self.device = device
self.parameters = parameters
self.__context__ = PipelineContext()
self.pipeline = None
self.connection = None
self.triggers = [] # actions can add trigger strings to the run a diagnostic
self.diagnostics = [DiagnoseNetwork]
self.timeout = None
self.timeout = timeout
self.protocols = []
self.compatibility = 2
# Was the job cleaned
Expand Down
21 changes: 14 additions & 7 deletions lava_dispatcher/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

# Bring in the strategy subclass lists, ignore pylint warnings.
# pylint: disable=unused-import
from __future__ import annotations

import lava_dispatcher.actions.boot.strategies
import lava_dispatcher.actions.deploy.strategies
import lava_dispatcher.actions.test.strategies
Expand Down Expand Up @@ -81,16 +83,22 @@ class JobParser:

# FIXME: needs a Schema and a check routine

def _timeouts(self, data, job):
if "job" in data.get("timeouts", {}):
duration = Timeout.parse(data["timeouts"]["job"])
job.timeout = Timeout("job", None, duration=duration)
@classmethod
def _parse_job_timeout(cls, data: dict) -> Timeout:
timeouts_dict = data["timeouts"]
duration = Timeout.parse(timeouts_dict["job"])
return Timeout("job", None, duration=duration)

def parse(self, content, device, job_id, logger, dispatcher_config, env_dut=None):
data = yaml_safe_load(content)
job = Job(job_id, data, logger)
job = Job(
job_id=job_id,
parameters=data,
logger=logger,
device=device,
timeout=self._parse_job_timeout(data),
)
test_counts = {}
job.device = device
job.parameters["env_dut"] = env_dut
# Load the dispatcher config
job.parameters["dispatcher"] = {}
Expand All @@ -106,7 +114,6 @@ def parse(self, content, device, job_id, logger, dispatcher_config, env_dut=None
for item in sorted(level_tuple, key=lambda level_tuple: level_tuple[1])
]
pipeline = Pipeline(job=job)
self._timeouts(data, job)

# deploy and boot classes can populate the pipeline differently depending
# on the test action type they are linked with (via namespacing).
Expand Down
5 changes: 5 additions & 0 deletions tests/lava_dispatcher/actions/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Copyright (C) 2023 Collabora Limited
#
# Author: Igor Ponomarev <[email protected]>
#
# SPDX-License-Identifier: GPL-2.0-or-later
5 changes: 5 additions & 0 deletions tests/lava_dispatcher/actions/boot/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Copyright (C) 2023 Collabora Limited
#
# Author: Igor Ponomarev <[email protected]>
#
# SPDX-License-Identifier: GPL-2.0-or-later
5 changes: 5 additions & 0 deletions tests/lava_dispatcher/actions/deploy/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Copyright (C) 2023 Collabora Limited
#
# Author: Igor Ponomarev <[email protected]>
#
# SPDX-License-Identifier: GPL-2.0-or-later
Loading

0 comments on commit c53746d

Please sign in to comment.