From 054c00fa841c3eeb6bdfa7f1e98a9199dcd5a78f Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Tue, 18 Oct 2022 14:33:59 +0200 Subject: [PATCH] storage: make GET /storage return the last successful probe when requested In the following patch: 4c59e1ee never return a PROBING response from guided_POST we fixed a UI crash occurring when a storage probing operation was ongoing while the user would enter the partitioning screen with "Use an entire disk". To address the issue, we made POST /storage/guided return the last (cached) successful probe result rather than returning a potential PROBING status result. That being said, when "Custom Storage Layout" is selected, we are using a different HTTP call: GET /storage which can also return a PROBING status result. So our fix was only partial. This patch adds a "use_cached_result" parameter (which defaults to false) to GET /storage. This makes the call return the last successful probe result. When the client selects "Custom Storage Layout" we now explicitly request a cached result so that we don't have to deal with a potential PROBING status response. Ideally, we should either: * make the client aware that a PROBING status can be returned and act accordingly. * pass the wait=true argument to GET /storage or POST /storage/guided But doing so has other implications and the kinetic release is imminent. Signed-off-by: Olivier Gayot --- subiquity/client/controllers/filesystem.py | 9 ++++++++- subiquity/common/apidef.py | 4 +++- subiquity/server/controllers/filesystem.py | 10 ++++++---- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/subiquity/client/controllers/filesystem.py b/subiquity/client/controllers/filesystem.py index 02c120f83..83b37b0a2 100644 --- a/subiquity/client/controllers/filesystem.py +++ b/subiquity/client/controllers/filesystem.py @@ -236,10 +236,17 @@ async def _answers_action(self, action): raise Exception("could not process action {}".format(action)) async def _guided_choice(self, choice): + # FIXME It would seem natural here to pass the wait=true flag to the + # below HTTP calls, especially because we wrap the coroutine in + # wait_with_progress. + # Having said that, making the server return a cached result seems like + # the least risky option to address https://launchpad.net/bugs/1993257 + # before the kinetic release. This is also similar to what we did for + # https://launchpad.net/bugs/1962205 if choice is not None: coro = self.endpoint.guided.POST(choice) else: - coro = self.endpoint.GET() + coro = self.endpoint.GET(use_cached_result=True) status = await self.app.wait_with_progress(coro) self.model = FilesystemModel(status.bootloader) self.model.load_server_data(status) diff --git a/subiquity/common/apidef.py b/subiquity/common/apidef.py index 85821a3dd..42f05fc35 100644 --- a/subiquity/common/apidef.py +++ b/subiquity/common/apidef.py @@ -250,7 +250,9 @@ def POST(data: Payload[GuidedChoice]) \ -> StorageResponse: pass - def GET(wait: bool = False) -> StorageResponse: ... + def GET(wait: bool = False, use_cached_result: bool = False) \ + -> StorageResponse: ... + def POST(config: Payload[list]): ... class reset: diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py index 0974d5371..22a4f81e3 100644 --- a/subiquity/server/controllers/filesystem.py +++ b/subiquity/server/controllers/filesystem.py @@ -293,10 +293,12 @@ def _done_response(self): dasd=self.model._probe_data.get('dasd', {}), storage_version=self.model.storage_version) - async def GET(self, wait: bool = False) -> StorageResponse: - probe_resp = await self._probe_response(wait, StorageResponse) - if probe_resp is not None: - return probe_resp + async def GET(self, wait: bool = False, use_cached_result: bool = False) \ + -> StorageResponse: + if not use_cached_result: + probe_resp = await self._probe_response(wait, StorageResponse) + if probe_resp is not None: + return probe_resp return self._done_response() async def POST(self, config: list):