Allow S3 Uploads while "Include Subdirectories" is False

[pbouill]

Can't upload to S3 if "target_dir" is false... simplify code to get the filename using os.path -- just get the file name directly without slicing the file name string (filename[len(target_dir) :]))

Was getting:

Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/motioneye/uploadservices.py", line 1307, in upload_media_file
    service.upload_file(target_dir, filename, camera_name)
  File "/usr/local/lib/python3.9/dist-packages/motioneye/uploadservices.py", line 1227, in upload_file
    s3.upload_file(filename, self._bucket, filename[len(target_dir) :])

[MichaIng]

Many thanks, makes perfectly sense. This actually makes target_dir obsolete, and using a sub directory anyway did never work with S3 as only a bucket and an object name can be passed. The switch to enable/disable upload with sub directories is however shown when selecting S3 upload, right? It then makes sense to hide the toggle like we do got Google Photos: https://github.com/motioneye-project/motioneye/blob/27971dc/motioneye/templates/main.html#L505-L509

Also here in the code, while the argument positions must not change, we can at least not assign target_dir but a dummy variable instead (is underscore common for this in Python?) and probably leave a code comment to not cause confusion among future contributors.

[zagrim]

I agree with Micha on also hiding the "Include subfolders" switch for S3 upload, so that we do not degrade the user experience with controls that do not cause any action or change. Otherwise, good catch and thanks for the PR.

I have no opinion regarding target_dir parameter in upload_file (camera_name was actually already not in use there, which my editor indicated readily), I'm ok with whatever practice for that that is used in Python.

[serniko97]

Some users, including myself, have the Movie File Name that includes also slashes to create subfolders automatically (for example motionEye/%Y-%m-%d/%Y-%m-%d_%H-%M-%S).
However it seems that the solution proposed in this PR (using os.path.basename(filename) instead) would only keep the last part of the file name.
If so it would be better to fix this before merging.

[MichaIng]

Cycling back here after a question regarding the filename appeared in #3192, and then checking #2912:

• So when using slashes in the filename, there are respective subdirectories generated at the server side?
• If so, "include subdirectories" would indeed work perfectly when just cloning the path separation of the original upload_file.
• The suggestion of copilot to override upload_data does not work, since that takes raw data as input to generate a network request, while boto3.upload_file takes a file path. But the logic can be cloned to split paths and assure there is no unexpected leading slash:

  if target_dir:
      target_dir = os.path.realpath(target_dir)
      rel_filename = os.path.realpath(filename)
      rel_filename = rel_filename[len(target_dir) :]
  
      while rel_filename.startswith('/'):
          rel_filename = rel_filename[1:]
  
  else:
      rel_filename = os.path.basename(filename)
  
  # Uploads the given file using a managed uploader, which will split up
  # large files automatically and upload parts in parallel.
  self.debug(f'uploading file "{filename}" to S3 bucket "{self._bucket}"')
  s3.upload_file(filename, self._bucket, rel_filename)
  

[MichaIng]

@pbouill @serniko97 could you test whether this solves it for you, both with and without included subdirs, and when adding those dirs via filename input? The method to strip leading slashes from the relative path, if subdirs shall be preserved, should be failsafe now.

[MichaIng]

Works well with MinIO. I could not replicate the leading slash issue with the old code either, but I guess it depends on the exact S3 server (version) whether it strips leading slashes automatically or not. At least it does not depend on boto3, since I can see the doubled slash (between bucket name and file path) in the debug output PUT URL, but MinIO seems to strip it automatically.

Anyway, the new code strips it correctly our end, which can be also seen in the logs, and it allows to disable subdirs, which as well works correctly, and which was the original purpose of this PR.