Skip to content

Commit

Permalink
storage: make GET /storage return the last successful probe when requ…
Browse files Browse the repository at this point in the history
…ested

In the following patch:

  4c59e1e 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 <[email protected]>
  • Loading branch information
ogayot committed Oct 18, 2022
1 parent b852a5d commit 054c00f
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
9 changes: 8 additions & 1 deletion subiquity/client/controllers/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion subiquity/common/apidef.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 6 additions & 4 deletions subiquity/server/controllers/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit 054c00f

Please sign in to comment.