diff --git a/binderhub/builder.py b/binderhub/builder.py index 1f2453c2b..ccb0df99e 100644 --- a/binderhub/builder.py +++ b/binderhub/builder.py @@ -19,7 +19,7 @@ from tornado.iostream import StreamClosedError from tornado.log import app_log from tornado.queues import Queue -from tornado.web import Finish, authenticated, HTTPError +from tornado.web import Finish, HTTPError, authenticated from .base import BaseHandler from .build import ProgressEvent @@ -411,15 +411,21 @@ async def get(self, provider_prefix, _unescaped_spec): require_build_only = self.settings.get("require_build_only", False) build_only = False if not require_build_only: - build_only_query_parameter = self.get_query_argument(name="build_only", default="") + build_only_query_parameter = self.get_query_argument( + name="build_only", default="" + ) if build_only_query_parameter.lower() == "true": - raise HTTPError(log_message="Building but not launching is not permitted!") + raise HTTPError( + log_message="Building but not launching is not permitted!" + ) else: # Not setting a default will make the function raise an error # if the `build_only` query parameter is missing from the request build_only_query_parameter = self.get_query_argument(name="build_only") if build_only_query_parameter.lower() != "true": - raise HTTPError(log_message="The `build_only=true` query parameter is required!") + raise HTTPError( + log_message="The `build_only=true` query parameter is required!" + ) # If we're here, it means a build only deployment is required build_only = True diff --git a/binderhub/tests/test_build.py b/binderhub/tests/test_build.py index f9b6ae6f3..48a43adcd 100644 --- a/binderhub/tests/test_build.py +++ b/binderhub/tests/test_build.py @@ -12,6 +12,7 @@ from kubernetes import client from tornado.httputil import url_concat from tornado.queues import Queue +from tornado.web import HTTPError from binderhub.build import KubernetesBuildExecutor, ProgressEvent from binderhub.build_local import LocalRepo2dockerBuild, ProcessTerminated, _execute_cmd @@ -122,46 +123,31 @@ async def test_build_fail(app, needs_build, needs_launch, always_build, pytestco @pytest.mark.asyncio(timeout=120) @pytest.mark.remote -async def test_build_no_launch(app): +async def test_build_no_launch_failing(app): """ - Test build endpoint with no launch option active, passed as a request query param. + Test build endpoint with build_only launch option active, + passed as a request query param, + but `require_build_only = False`. """ slug = "gh/binderhub-ci-repos/cached-minimal-dockerfile/HEAD" build_url = f"{app.url}/build/{slug}" params = {} - params = {"no-launch": "True"} + params = {"build_only": "true"} r = await async_requests.get(build_url, stream=True, params=params) r.raise_for_status() - - events = [] - launch_events = 0 + failed_events=0 async for line in async_requests.iter_lines(r): line = line.decode("utf8", "replace") if line.startswith("data:"): event = json.loads(line.split(":", 1)[1]) - events.append(event) - assert "message" in event - sys.stdout.write(f"{event.get('phase', '')}: {event['message']}") - if event.get("phase") == "info": - assert ( - "Found no launch option. Image will not be launched after build" - in event["message"] - ) - # This is the signal that the image was built. - # Check the message for clue that the image won't be launched and - # break out of the loop now because BinderHub keeps the connection open - # for many seconds after to avoid "reconnects" from slow clients - if event.get("phase") == "ready": - assert "Image won't be launched" in event["message"] - r.close() + assert event.get("phase") not in ("launching", "ready") + if event.get("phase") == "failed": + failed_events += 1 + assert "Building but not launching is not permitted!" in event["message"] break - if event.get("phase") == "launching": - launch_events += 1 + r.close() - assert launch_events == 0 - final = events[-1] - assert "phase" in final - assert final["phase"] == "ready" + assert failed_events > 0, "Should have seen phase 'failed'" def _list_image_builder_pods_mock():