Merge pull request #846 from OpenTrafficCam/task/10323-save-and-reopen-a-project-in-s3-mode

Task/10323 save and reopen a project in s3 mode

Author: briemla

OTAnalytics/adapter_ui/dummy_viewmodel.py

@@ -627,6 +627,12 @@ def show_current_project(self, _: Any = None) -> None:
 
     async def save_otconfig(self) -> None:
         save_suggestion = self._application.suggest_save_path(OTCONFIG_FILE_TYPE)
+        if self._application.get_current_key_prefix() is not None:
+            # A project that names a location was opened by an operator naming
+            # it, not chosen from a picker (OP#10323 decision 1), so it always
+            # re-saves to its own name and place with no new UI.
+            await self._save_otconfig(save_suggestion.file_path)
+            return
         configuration_file = await self._ui_factory.ask_for_save_file_path(
             title="Save configuration as",
             filetypes=[(f"{OTCONFIG_FILE_TYPE} file", f"*.{OTCONFIG_FILE_TYPE}")],
@@ -636,9 +642,9 @@ async def save_otconfig(self) -> None:
         )
         if not configuration_file:
             return
-        self._save_otconfig(configuration_file)
+        await self._save_otconfig(configuration_file)
 
-    def _save_otconfig(self, otconfig_file: Path) -> None:
+    async def _save_otconfig(self, otconfig_file: Path) -> None:
         # Ensure the file has the correct extension
         otconfig_file_str = ensure_file_extension_is_present(
             str(otconfig_file), [f"*.{OTCONFIG_FILE_TYPE}"], f".{OTCONFIG_FILE_TYPE}"
@@ -647,7 +653,7 @@ def _save_otconfig(self, otconfig_file: Path) -> None:
 
         logger().info(f"Config file to save: {otconfig_file}")
         try:
-            self._application.save_otconfig(otconfig_file)
+            await self._application.save_otconfig(otconfig_file)
         except NoSectionsToSave:
             message = (
                 f"{MESSAGE_CONFIGURATION_NOT_SAVED}"
@@ -943,6 +949,12 @@ def _load_otflow(self, otflow_file: Path) -> None:
 
     async def save_configuration(self) -> None:
         save_suggestion = self._application.suggest_save_path(OTCONFIG_FILE_TYPE)
+        if self._application.get_current_key_prefix() is not None:
+            # A project that names a location is always an otconfig and always
+            # re-saves to its own name and place with no new UI (OP#10323
+            # decision 1 & 5).
+            await self._save_otconfig(save_suggestion.file_path)
+            return
         configuration_file = await self._ui_factory.ask_for_save_file_path(
             title="Save configuration as",
             filetypes=[
@@ -967,13 +979,13 @@ async def save_configuration(self) -> None:
         if configuration_file.suffix == f".{OTFLOW_FILE_TYPE}":
             self._save_otflow(configuration_file)
         elif configuration_file.suffix == f".{OTCONFIG_FILE_TYPE}":
-            self._save_otconfig(configuration_file)
+            await self._save_otconfig(configuration_file)
         else:
             raise ValueError("Configuration file to save has unknown file extension")
 
     async def quick_save_configuration(self) -> None:
         try:
-            self._application.quick_save_configuration()
+            await self._application.quick_save_configuration()
         except NoExistingFileToSave:
             await self.save_configuration()
 

OTAnalytics/application/application.py

@@ -8,9 +8,11 @@
     ExportFormat,
 )
 from OTAnalytics.application.datastore import Datastore
+from OTAnalytics.application.key_prefix import S3KeyPrefix
 from OTAnalytics.application.project import SvzMetadata
 from OTAnalytics.application.state import (
     ActionState,
+    CurrentKeyPrefix,
     FileState,
     FlowState,
     SectionState,
@@ -157,6 +159,7 @@ def __init__(
         export_track_statistics: ExportTrackStatistics,
         get_current_remark: GetCurrentRemark,
         update_count_plots: CountPlotsUpdater,
+        current_key_prefix: CurrentKeyPrefix,
     ) -> None:
         self._datastore: Datastore = datastore
         self.track_state: TrackState = track_state
@@ -204,6 +207,7 @@ def __init__(
         self._export_track_statistics = export_track_statistics
         self._get_current_remark = get_current_remark
         self._update_count_plots = update_count_plots
+        self._current_key_prefix = current_key_prefix
 
     def connect_observers(self) -> None:
         """
@@ -305,8 +309,16 @@ def remove_flow(self, flow_id: FlowId) -> None:
     def update_flow(self, flow: Flow) -> None:
         self._datastore.update_flow(flow)
 
-    def save_otconfig(self, file: Path) -> None:
-        self._save_otconfig(file)
+    async def save_otconfig(self, file: Path) -> None:
+        await self._save_otconfig(file)
+
+    def get_current_key_prefix(self) -> S3KeyPrefix | None:
+        """Where the current project's data lives, or None outside s3 mode.
+
+        A project naming a prefix is how the save path knows to skip the
+        file-picker dialog: see OP#10323.
+        """
+        return self._current_key_prefix.get()
 
     def load_otconfig(self, file: Path) -> None:
         """Load an otconfig, blocking. Outside an event loop only."""
@@ -675,8 +687,8 @@ def update_svz_metadata(self, metadata: SvzMetadata) -> None:
     def get_track_repository_size(self) -> int:
         return self._track_repository_size.get()
 
-    def quick_save_configuration(self) -> None:
-        self._quick_save_configuration.save()
+    async def quick_save_configuration(self) -> None:
+        await self._quick_save_configuration.save()
 
     def config_has_changed(self) -> bool:
         return self._config_has_changed.has_changed()

OTAnalytics/application/upload_otconfig.py

@@ -0,0 +1,24 @@
+from abc import ABC, abstractmethod
+from pathlib import Path
+
+from OTAnalytics.application.key_prefix import S3KeyPrefix
+
+
+class UploadOtconfig(ABC):
+    """Puts a saved otconfig where a later s3-mode load can find it again.
+
+    Nothing here asks what the transfer mode is: the wiring picks an
+    implementation, and the implementation is the answer. See ADR 0004 and
+    OP#10323's decision log.
+    """
+
+    @abstractmethod
+    async def upload(self, file: Path, key_prefix: S3KeyPrefix) -> None:
+        raise NotImplementedError
+
+
+class NoOtconfigUpload(UploadOtconfig):
+    """Wired whenever s3 is not configured; saving stays purely local."""
+
+    async def upload(self, file: Path, key_prefix: S3KeyPrefix) -> None:
+        return None

OTAnalytics/application/use_cases/config.py

@@ -1,8 +1,10 @@
 from pathlib import Path
 
 from OTAnalytics.application.datastore import Datastore
+from OTAnalytics.application.logger import logger
 from OTAnalytics.application.parser.config_parser import ConfigParser
 from OTAnalytics.application.state import ConfigurationFile, CurrentKeyPrefix, FileState
+from OTAnalytics.application.upload_otconfig import UploadOtconfig
 from OTAnalytics.application.use_cases.get_current_remark import GetCurrentRemark
 
 
@@ -22,14 +24,16 @@ def __init__(
         state: FileState,
         get_current_remark: GetCurrentRemark,
         current_key_prefix: CurrentKeyPrefix,
+        otconfig_upload: UploadOtconfig,
     ) -> None:
         self._datastore = datastore
         self._config_parser = config_parser
         self._state = state
         self._get_current_remark = get_current_remark
         self._current_key_prefix = current_key_prefix
+        self._otconfig_upload = otconfig_upload
 
-    def __call__(self, file: Path) -> None:
+    async def __call__(self, file: Path) -> None:
         project = self._datastore.project
 
         # Collect all validation errors
@@ -50,6 +54,7 @@ def __call__(self, file: Path) -> None:
         # The same value reaches `convert` below. Passing it to only one of the
         # two would make every project report itself as permanently unsaved.
         s3_key_prefix = self._current_key_prefix.get()
+        logger().info(f"Saving otconfig to '{file}'")
         self._config_parser.serialize(
             project=project,
             video_files=video_files,
@@ -75,3 +80,6 @@ def __call__(self, file: Path) -> None:
                 ),
             )
         )
+        if s3_key_prefix is not None:
+            logger().info(f"Uploading otconfig '{file}' to s3 under '{s3_key_prefix}'")
+            await self._otconfig_upload.upload(file, s3_key_prefix)

OTAnalytics/application/use_cases/quick_save_configuration.py

@@ -14,14 +14,14 @@ def __init__(
         self._save_otflow = save_otflow
         self._save_otconfig = save_otconfig
 
-    def save(self) -> None:
+    async def save(self) -> None:
         if not (config := self._state.last_saved_config.get()):
             raise NoExistingFileToSave("No saved file to save configuration to")
 
         if config.is_otflow:
             self._save_otflow.save(config.file)
         elif config.is_otconfig:
-            self._save_otconfig(config.file)
+            await self._save_otconfig(config.file)
         else:
             raise UnsupportedConfiguration(
                 "Unable to save configuration. "

OTAnalytics/plugin_parser/otconfig_parser.py

@@ -384,6 +384,13 @@ def convert(
     ) -> dict:
         parent_folder = file.parent
         project_content = project.to_dict()
+        if s3_key_prefix is not None:
+            # Data is picked per session by time range (OP#10283), so a
+            # project's own file references have no reader in s3 mode.
+            # Recording them would only leave stale paths into the ephemeral
+            # `user_source` staging area behind. See OP#10323's decision log.
+            video_files = []
+            track_files = []
         video_content = self._video_parser.convert(
             video_files,
             relative_to=parent_folder,

OTAnalytics/plugin_s3/otconfig_upload.py

@@ -0,0 +1,15 @@
+from pathlib import Path
+
+from OTAnalytics.application.key_prefix import S3KeyPrefix
+from OTAnalytics.application.upload_otconfig import UploadOtconfig
+from OTAnalytics.plugin_s3.upload import S3Upload
+
+
+class S3OtconfigUpload(UploadOtconfig):
+    """Uploads a saved otconfig beside the project's other objects in s3."""
+
+    def __init__(self, upload: S3Upload) -> None:
+        self._upload = upload
+
+    async def upload(self, file: Path, key_prefix: S3KeyPrefix) -> None:
+        await self._upload.upload(src=file, key=f"{key_prefix}/{file.name}")

OTAnalytics/plugin_ui/base_application.py

@@ -58,6 +58,7 @@
     SwitchToNext,
     SwitchToPrevious,
 )
+from OTAnalytics.application.upload_otconfig import NoOtconfigUpload, UploadOtconfig
 from OTAnalytics.application.use_cases.add_new_remark import AddNewRemark
 from OTAnalytics.application.use_cases.apply_cli_cuts import ApplyCliCuts
 from OTAnalytics.application.use_cases.assignment_repository import (
@@ -436,8 +437,18 @@ def save_otconfig(self) -> SaveOtconfig:
             self.file_state,
             self.get_current_remark,
             self.current_key_prefix,
+            self.otconfig_upload,
         )
 
+    @cached_property
+    def otconfig_upload(self) -> UploadOtconfig:
+        """Puts a saved otconfig where s3 mode can find it again.
+
+        Overridden where s3 can be configured. This starter never sees a
+        project naming a location, so saving never needs to upload anywhere.
+        """
+        return NoOtconfigUpload()
+
     @cached_property
     def current_key_prefix(self) -> CurrentKeyPrefix:
         """One holder for the whole application.

OTAnalytics/plugin_ui/gui_application.py

@@ -209,6 +209,7 @@ def application(self) -> OTAnalyticsApplication:
             self.export_track_statistics,
             self.get_current_remark,
             self.update_count_plots,
+            self.current_key_prefix,
         )
 
     @cached_property

OTAnalytics/plugin_ui/nicegui_application.py

@@ -13,6 +13,7 @@
     RefuseAnyProjectLocation,
     ValidateProjectLocation,
 )
+from OTAnalytics.application.upload_otconfig import NoOtconfigUpload, UploadOtconfig
 from OTAnalytics.application.use_cases.ask_for_load_window import AskForLoadWindow
 from OTAnalytics.application.use_cases.provide_input_files import (
     ProvideTrackFiles,
@@ -32,12 +33,14 @@
 from OTAnalytics.plugin_s3.download import S3Download
 from OTAnalytics.plugin_s3.download_objects import DownloadObjects
 from OTAnalytics.plugin_s3.list_objects import S3ListObjects
+from OTAnalytics.plugin_s3.otconfig_upload import S3OtconfigUpload
 from OTAnalytics.plugin_s3.project_location import RequireWellFormedKeyPrefix
 from OTAnalytics.plugin_s3.s3_file_providers import (
     S3TrackFileProvider,
     S3VideoFileProvider,
 )
 from OTAnalytics.plugin_s3.store import S3Store
+from OTAnalytics.plugin_s3.upload import S3Upload
 from OTAnalytics.plugin_ui.gui_application import OtAnalyticsGuiApplicationStarter
 from OTAnalytics.plugin_ui.nicegui_gui.nicegui.progressbar import (
     NiceguiProgressbarBuilder,
@@ -328,6 +331,12 @@ def validate_project_location(self) -> ValidateProjectLocation:
             return RequireWellFormedKeyPrefix()
         return RefuseAnyProjectLocation()
 
+    @cached_property
+    def otconfig_upload(self) -> UploadOtconfig:
+        if self.s3_config:
+            return S3OtconfigUpload(S3Upload(self.s3_store))
+        return NoOtconfigUpload()
+
     @cached_property
     def reset_application(self) -> ResetApplication:
         """Resetting a project also frees the staged downloads it used.