Skip to content

Commit

Permalink
fix(remote-build): parse --launchpad-timeout argument (#4426)
Browse files Browse the repository at this point in the history
Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
  • Loading branch information
mr-cal committed Oct 28, 2023
1 parent e3b8a3a commit c6f41da
Show file tree
Hide file tree
Showing 8 changed files with 191 additions and 25 deletions.
16 changes: 15 additions & 1 deletion snapcraft/commands/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,13 @@ class RemoteBuildCommand(BaseCommand):
option, followed by the build number informed when the remote
build was originally dispatched. The current state of the
remote build for each architecture can be checked using the
--status option."""
--status option.
To set a timeout on the remote-build command, use the option
``--launchpad-timeout=<seconds>``. The timeout is local, so the build on
launchpad will continue even if the local instance of snapcraft is
interrupted or times out.
"""
)

@overrides
Expand Down Expand Up @@ -101,6 +107,13 @@ def fill_parser(self, parser: argparse.ArgumentParser) -> None:
action="store_true",
help="acknowledge that uploaded code will be publicly available.",
)
parser.add_argument(
"--launchpad-timeout",
type=int,
default=0,
metavar="<seconds>",
help="Time in seconds to wait for launchpad to build.",
)

@overrides
def run(self, parsed_args: argparse.Namespace) -> None:
Expand Down Expand Up @@ -222,6 +235,7 @@ def _run_new_remote_build(self) -> None:
project_name=self._get_project_name(),
architectures=self._determine_architectures(),
project_dir=Path(),
timeout=self._parsed_args.launchpad_timeout,
)

if self._parsed_args.status:
Expand Down
10 changes: 7 additions & 3 deletions snapcraft/remote/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,14 @@ def __init__(self, message: str) -> None:
class RemoteBuildTimeoutError(RemoteBuildError):
"""Remote-build timed out."""

def __init__(self) -> None:
brief = "Remote build timed out."
def __init__(self, recovery_command: str) -> None:
brief = "Remote build command timed out."
details = (
"Build may still be running on Launchpad and can be recovered "
f"with {recovery_command!r}."
)

super().__init__(brief=brief)
super().__init__(brief=brief, details=details)


class LaunchpadHttpsError(RemoteBuildError):
Expand Down
25 changes: 20 additions & 5 deletions snapcraft/remote/launchpad.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,14 @@ def _get_url_basename(url: str):


class LaunchpadClient:
"""Launchpad remote builder operations."""
"""Launchpad remote builder operations.
:param app_name: Name of the application.
:param build_id: Unique identifier for the build.
:param project_name: Name of the project.
:param architectures: List of architectures to build on.
:param timeout: Time in seconds to wait for the build to complete.
"""

def __init__(
self,
Expand All @@ -88,7 +95,7 @@ def __init__(
build_id: str,
project_name: str,
architectures: Sequence[str],
deadline: int = 0,
timeout: int = 0,
) -> None:
self._app_name = app_name

Expand All @@ -104,7 +111,11 @@ def __init__(
self._lp: Launchpad = self._login()
self.user = self._lp.me.name # type: ignore

self._deadline = deadline
# calculate deadline from the timeout
if timeout > 0:
self._deadline = int(time.time()) + timeout
else:
self._deadline = 0

@property
def architectures(self) -> Sequence[str]:
Expand Down Expand Up @@ -135,7 +146,11 @@ def _check_timeout_deadline(self) -> None:
return

if int(time.time()) >= self._deadline:
raise errors.RemoteBuildTimeoutError()
raise errors.RemoteBuildTimeoutError(
recovery_command=(
f"{self._app_name} remote-build --recover --build-id {self._build_id}"
)
)

def _create_data_directory(self) -> Path:
data_dir = Path(
Expand Down Expand Up @@ -199,7 +214,7 @@ def _lp_load_url(self, url: str) -> Entry:
return self._lp.load(url)

def _wait_for_build_request_acceptance(self, build_request: Entry) -> None:
# Not be be confused with the actual build(s), this is
# Not to be confused with the actual build(s), this is
# ensuring that Launchpad accepts the build request.
while build_request.status == "Pending":
# Check to see if we've run out of time.
Expand Down
5 changes: 4 additions & 1 deletion snapcraft/remote/remote_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,21 @@ class RemoteBuilder:
:param project_name: Name of the project.
:param architectures: List of architectures to build on.
:param project_dir: Path of the project.
:param timeout: Time in seconds to wait for the build to complete.
:raises UnsupportedArchitectureError: if any architecture is not supported
for remote building.
:raises LaunchpadHttpsError: If a connection to Launchpad cannot be established.
"""

def __init__(
def __init__( # noqa: PLR0913 pylint: disable=too-many-arguments
self,
app_name: str,
build_id: Optional[str],
project_name: str,
architectures: List[str],
project_dir: Path,
timeout: int,
):
self._app_name = app_name
self._project_name = project_name
Expand Down Expand Up @@ -78,6 +80,7 @@ def __init__(
build_id=self._build_id,
project_name=self._project_name,
architectures=self._architectures,
timeout=timeout,
)

@property
Expand Down
49 changes: 49 additions & 0 deletions tests/unit/commands/test_remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,50 @@ def test_cannot_load_snapcraft_yaml(capsys):
)


@pytest.mark.parametrize(
"create_snapcraft_yaml", CURRENT_BASES | LEGACY_BASES, indirect=True
)
@pytest.mark.usefixtures(
"create_snapcraft_yaml", "mock_confirm", "use_new_remote_build", "mock_argv"
)
def test_launchpad_timeout_default(mock_remote_builder):
"""Use the default timeout `0` when `--launchpad-timeout` is not provided."""
cli.run()

mock_remote_builder.assert_called_with(
app_name="snapcraft",
build_id=None,
project_name="mytest",
architectures=ANY,
project_dir=Path(),
timeout=0,
)


@pytest.mark.parametrize(
"create_snapcraft_yaml", CURRENT_BASES | LEGACY_BASES, indirect=True
)
@pytest.mark.usefixtures(
"create_snapcraft_yaml", "mock_confirm", "use_new_remote_build"
)
def test_launchpad_timeout(mocker, mock_remote_builder):
"""Pass the `--launchpad-timeout` to the remote builder."""
mocker.patch.object(
sys, "argv", ["snapcraft", "remote-build", "--launchpad-timeout", "100"]
)

cli.run()

mock_remote_builder.assert_called_with(
app_name="snapcraft",
build_id=None,
project_name="mytest",
architectures=ANY,
project_dir=Path(),
timeout=100,
)


################################
# Snapcraft project base tests #
################################
Expand Down Expand Up @@ -537,6 +581,7 @@ def test_determine_architectures_from_snapcraft_yaml(
project_name="mytest",
architectures=expected_archs,
project_dir=Path(),
timeout=0,
)


Expand All @@ -560,6 +605,7 @@ def test_determine_architectures_host_arch(mocker, mock_remote_builder):
project_name="mytest",
architectures=["arm64"],
project_dir=Path(),
timeout=0,
)


Expand Down Expand Up @@ -592,6 +638,7 @@ def test_determine_architectures_provided_by_user(
project_name="mytest",
architectures=expected_archs,
project_dir=Path(),
timeout=0,
)


Expand Down Expand Up @@ -640,6 +687,7 @@ def test_build_id_provided(mocker, mock_remote_builder):
project_name="mytest",
architectures=ANY,
project_dir=Path(),
timeout=0,
)


Expand All @@ -660,6 +708,7 @@ def test_build_id_not_provided(mock_remote_builder):
project_name="mytest",
architectures=ANY,
project_dir=Path(),
timeout=0,
)


Expand Down
22 changes: 16 additions & 6 deletions tests/unit/remote/test_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,24 @@ def test_git_error():

def test_remote_build_timeout_error():
"""Test RemoteBuildTimeoutError."""
error = errors.RemoteBuildTimeoutError()
error = errors.RemoteBuildTimeoutError(
recovery_command="craftapp remote-build --recover --build-id test-id"
)

assert str(error) == "Remote build timed out."
assert (
repr(error)
== "RemoteBuildTimeoutError(brief='Remote build timed out.', details=None)"
assert str(error) == (
"Remote build command timed out.\nBuild may still be running on Launchpad and "
"can be recovered with 'craftapp remote-build --recover --build-id test-id'."
)
assert repr(error) == (
"RemoteBuildTimeoutError(brief='Remote build command timed out.', "
'details="Build may still be running on Launchpad and can be recovered with '
"'craftapp remote-build --recover --build-id test-id'.\")"
)
assert error.brief == "Remote build command timed out."
assert error.details == (
"Build may still be running on Launchpad and can be recovered with "
"'craftapp remote-build --recover --build-id test-id'."
)
assert error.brief == "Remote build timed out."


def test_launchpad_https_error():
Expand Down
83 changes: 74 additions & 9 deletions tests/unit/remote/test_launchpad.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,18 +353,55 @@ def test_start_build_error(mocker, launchpad_client):
assert str(raised.value) == "snapcraft.yaml not found..."


def test_start_build_error_timeout(mocker, launchpad_client):
def test_start_build_deadline_not_reached(mock_login_with, mocker):
"""Do not raise an error if the deadline has not been reached."""

def lp_refresh(self):
"""Update the status from Pending to Completed when refreshed."""
self.status = "Completed"

mocker.patch(
"tests.unit.remote.test_launchpad.SnapImpl.requestBuilds",
return_value=SnapBuildReqImpl(status="Pending", error_message=""),
)
mocker.patch.object(SnapBuildReqImpl, "lp_refresh", lp_refresh)
mocker.patch("time.time", return_value=500)
launchpad_client._deadline = 499

lpc = LaunchpadClient(
app_name="test-app",
build_id="id",
project_name="test-project",
architectures=[],
timeout=100,
)

lpc.start_build()


def test_start_build_timeout_error(mock_login_with, mocker):
"""Raise an error if the build times out."""
mocker.patch(
"tests.unit.remote.test_launchpad.SnapImpl.requestBuilds",
return_value=SnapBuildReqImpl(status="Pending", error_message=""),
)
mocker.patch("time.time", return_value=500)
lpc = LaunchpadClient(
app_name="test-app",
build_id="id",
project_name="test-project",
architectures=[],
timeout=100,
)
# advance 1 second past deadline
mocker.patch("time.time", return_value=601)

with pytest.raises(errors.RemoteBuildTimeoutError) as raised:
launchpad_client.start_build()
lpc.start_build()

assert str(raised.value) == "Remote build timed out."
assert str(raised.value) == (
"Remote build command timed out.\nBuild may still be running on Launchpad and "
"can be recovered with 'test-app remote-build --recover --build-id id'."
)


def test_issue_build_request_defaults(launchpad_client):
Expand Down Expand Up @@ -422,16 +459,44 @@ def test_monitor_build_error(mock_log, mock_download_file, mocker, launchpad_cli
]


def test_monitor_build_error_timeout(mocker, launchpad_client):
def test_monitor_build_deadline_not_reached(mock_login_with, mocker):
"""Do not raise an error if the deadline has not been reached."""
mocker.patch("snapcraft.remote.LaunchpadClient._download_file")
lpc = LaunchpadClient(
app_name="test-app",
build_id="id",
project_name="test-project",
architectures=[],
timeout=100,
)

lpc.start_build()
lpc.monitor_build(interval=0)


def test_monitor_build_timeout_error(mock_login_with, mocker):
"""Raise an error if the build times out."""
mocker.patch("snapcraft.remote.LaunchpadClient._download_file")
mocker.patch("time.time", return_value=500)
launchpad_client._deadline = 499
launchpad_client.start_build()
lpc = LaunchpadClient(
app_name="test-app",
build_id="id",
project_name="test-project",
architectures=[],
timeout=100,
)
# advance 1 second past deadline
mocker.patch("time.time", return_value=601)

lpc.start_build()

with pytest.raises(errors.RemoteBuildTimeoutError) as raised:
launchpad_client.monitor_build(interval=0)
lpc.monitor_build(interval=0)

assert str(raised.value) == "Remote build timed out."
assert str(raised.value) == (
"Remote build command timed out.\nBuild may still be running on Launchpad and "
"can be recovered with 'test-app remote-build --recover --build-id id'."
)


def test_get_build_status(launchpad_client):
Expand Down

0 comments on commit c6f41da

Please sign in to comment.