feat: Enhance progress outputs for image cache uploads

[mitchnegus]

Enhance progress outputs for image cache uploads

Description

In service of improving the user experience of launching FIREWHEEL experiments, this MR begins adding more functionality to the ModelComponent and ModelComponentManager objects.

This MR contains a few substantive changes:

1. Most obvious to the user, the output from loading images to the image cache has been expanded. Images are grouped by MC, and while the load time continues to display independently per-image, the spinner is now associated with a progress output of the images to be loaded for any given MC. Also, large VM resources (over 250 MB) now also receive progress bars during upload.
2. The change described in (1) arose from needing to use a distinct layout of renderables from the rich library to separately display image cache upload progress, experiment graph progress (not implemented in this MR), experiment time progress (also not implemented in this MR), and the outputs of MC plugins. On the backend, that meant compartmentalizing image uploads to be represented by progress bars to match the live progress example provided by rich, as well as adjusting some of the image handling to know before creating the progress bar whether an image would need updating.
3. In the process of accomplishing (2), the ModelComponent object was heavily refactored. It handled VM resources and images using very similar, but entirely separate upload processes. Now, it performs uploads in a uniform fashion, depending on the subtleties of each upload type defined via a new object class—a _ModelComponentFile. This object is notionally analagous to the existing FileStoreFile object from the lib.minimega.file_store module, but this represents the file as part of the model component rather than the file store. Ultimately, it seems like these file-like objects might do well to be combined eventually, but that feels like a separate issue.

Type of Change

Please select the type of change your pull request introduces:

• Bugfix
• New feature
• Documentation update
• Other (please describe):

Checklist

• This PR conforms to the process detailed in the Contributing Guide.
• I have included no proprietary/sensitive information in my code.
• I have performed a self-review of my code.
• I have commented my code, particularly in hard-to-understand areas.
• I have made corresponding changes to the documentation.
• My changes generate no new warnings.
• I have tested my code.

[mitchnegus]

@sdelliot I mentioned it in response to your comments above, but I cleaned up the module a fair bit. The issues you rose were all tied to that weird return value construct, and I found that the easiest way to clean things up was to use your enum suggestion paired with a couple new objects representing the images and VM resources.

There are still 4 tests that fail. I held off on fixing these because I wanted suggestions on solving them. The ModelComponent object follows this logic:

1. No image in file store, upload it.
2. If it exists, compare times. Replace file store images with newer MC images that are not duplicates.

This leaves some undefined behavior in other cases—such as when an MC image was older than the file store image... One of the currently failing test cases suggest that this might happen if the user were to copy the MC version of an image elsewhere, then replace it and reupload it (maybe for debugging something), and finally reverting it back to the original and reuploading that again. Previously, we seemed to just (silently) replace the newer file store image in this case.

My current draft of this takes an opposite approach, and rather than deleting the newer file store image, it raises a user warning that the cache is out of sync. The user can then decide to blow away the cache, or investigate, or whatever. Getting into this state seems like a pretty weird case that only developers might ever actually be intendng to reach, but even if the best solution here for most users is almost always to just reset the cache, silently deleting files feels to me like a big no-no.

If that feels like the right approach, I can add, update, and/or remove tests to match. Otherwise, I'm open to other ideas.

Side note: A cool benefit of this round of refactoring is that VM resources can now also easily receive progress bars. I set it so that large VMRs (over 250 MB) will get them when they're uploaded to the cache. I think that feature had been requested out-of-band.

[sdelliot]

@sdelliot I mentioned it in response to your comments above, but I cleaned up the module a fair bit. The issues you rose were all tied to that weird return value construct, and I found that the easiest way to clean things up was to use your enum suggestion paired with a couple new objects representing the images and VM resources.

There are still 4 tests that fail. I held off on fixing these because I wanted suggestions on solving them. The ModelComponent object follows this logic:

1. No image in file store, upload it.

2. If it exists, compare times. Replace file store images with newer MC images that are not duplicates.

This leaves some undefined behavior in other cases—such as when an MC image was older than the file store image... One of the currently failing test cases suggest that this might happen if the user were to copy the MC version of an image elsewhere, then replace it and reupload it (maybe for debugging something), and finally reverting it back to the original and reuploading that again. Previously, we seemed to just (silently) replace the newer file store image in this case.

My current draft of this takes an opposite approach, and rather than deleting the newer file store image, it raises a user warning that the cache is out of sync. The user can then decide to blow away the cache, or investigate, or whatever. Getting into this state seems like a pretty weird case that only developers might ever actually be intendng to reach, but even if the best solution here for most users is almost always to just reset the cache, silently deleting files feels to me like a big no-no.

If that feels like the right approach, I can add, update, and/or remove tests to match. Otherwise, I'm open to other ideas.

Side note: A cool benefit of this round of refactoring is that VM resources can now also easily receive progress bars. I set it so that large VMRs (over 250 MB) will get them when they're uploaded to the cache. I think that feature had been requested out-of-band.

That is certainly one of the reasons why we had that code in there originally. The second is for the case of using vm builder pointing to the image in the cache. In the case that you accidentally modify the image, you wouldn't want that for your experiment.

I agree that it doesn't seem valuable or wise to just silently delete the newer images and I am on board with your approach. If a user is messing around with the images that much, they likely wouldn't want their image deleted and maybe would want to save it off.

The new Enum structure really cleans up the code a lot! I also agree that we can likely merge the FileStoreFile and _ModelComponentFile, but I am okay with punting that down the road.