Skip to content

Web UI: Upload to tmp file name then rename if successful#1272

Merged
rdmark merged 2 commits intodevelopfrom
rdmark-issue-1270
Oct 31, 2023
Merged

Web UI: Upload to tmp file name then rename if successful#1272
rdmark merged 2 commits intodevelopfrom
rdmark-issue-1270

Conversation

@rdmark
Copy link
Copy Markdown
Member

@rdmark rdmark commented Oct 28, 2023

Additionally, in order to simplify access to the file commands class in the file upload function, refactoring associated code to:

  1. Move the dropzone.js operations back into web.py
  2. Move list_images() from file commands into piscsi commands (it was the only class method in that package that calls the protobuf interface)

@rdmark rdmark force-pushed the rdmark-issue-1270 branch 2 times, most recently from b354733 to 70c7184 Compare October 28, 2023 11:13
@rdmark rdmark requested a review from nucleogenic October 28, 2023 11:29
@rdmark rdmark force-pushed the rdmark-issue-1270 branch from 70c7184 to c9468a1 Compare October 28, 2023 11:34
@rdmark rdmark changed the title Upload to tmp file name then rename if successful Web UI: Upload to tmp file name then rename if successful Oct 29, 2023
try:
archive_info = self._get_archive_info(
f"{server_info['image_dir']}/{file.name}",
_cache_extra_key=file.size,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I remember what this is for now, which was a somewhat brittle cache invalidator for when a file is replaced with another of the same name. Otherwise, we'll return the previous file.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha! I was confused because _cache_extra_key wasn't actually called in our code. So I assume it's used by the cache library behind the scenes then?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restored those two lines

@rdmark rdmark force-pushed the rdmark-issue-1270 branch from c9468a1 to bed15f6 Compare October 30, 2023 00:02

def __init__(self, sock_cmd: SocketCmds, piscsi: PiscsiCmds, token=None, locale=None):
self.sock_cmd = sock_cmd
def __init__(self, piscsi: PiscsiCmds):
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nucleogenic Question! What do you think about not initializing the class with the PiscsiCmds object? The original motivation was for sharing the token and locale, I believe. But now when FileCmds doesn't access the protobuf interface anymore it's not really needed, is it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind; I gave it a go and quickly realized that instantiating a new PiscsiCmds object is a pain.

@rdmark rdmark merged commit 029cf06 into develop Oct 31, 2023
@rdmark rdmark deleted the rdmark-issue-1270 branch October 31, 2023 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants