-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow postprocessing binary/script post download #180
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, I like this feature as it opens up a lot of possibilities. Only minor adjustments needed
The binary / executable script receives a first argument with the storage location for the downloaded video. You can easily mount a script from a local filesystem to the container: | ||
|
||
```bash | ||
rm -r /tmp/unifi ; docker rmi ghcr.io/ep1cman/unifi-protect-backup ; poetry build && docker buildx build . -t ghcr.io/ep1cman/unifi-protect-backup ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rebuilding the container seems excessive for the readme
@@ -93,7 +103,7 @@ async def start(self): | |||
except Exception as e: | |||
self.logger.error(f"Unexpected exception occurred, abandoning event {event.id}:", exc_info=e) | |||
|
|||
async def _upload_video(self, video: bytes, destination: pathlib.Path, rclone_args: str): | |||
async def _upload_video(self, video: bytes, destination: pathlib.Path, rclone_args: str, postprocess_binary: str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this passed into here and never used?
@@ -103,6 +113,7 @@ async def _upload_video(self, video: bytes, destination: pathlib.Path, rclone_ar | |||
video (bytes): The data to be written to the file | |||
destination (pathlib.Path): Where rclone should write the file | |||
rclone_args (str): Optional extra arguments to pass to `rclone` | |||
postprocess_binary (str): Optional extra path to postprocessing binary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above comment
@@ -82,9 +85,16 @@ async def start(self): | |||
self.logger.debug(f" Destination: {destination}") | |||
|
|||
try: | |||
await self._upload_video(video, destination, self._rclone_args) | |||
await self._upload_video(video, destination, self._rclone_args, self._postprocess_binary) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this passed into here and never used?
|
||
# Postprocess | ||
if self._postprocess_binary: | ||
returncode_postprocess, stdout_postprocess, stderr_postprocess = await run_command(f'"{self._postprocess_binary}" "{destination}"') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel that the exit codes here should have some signficance to how the script proceeds.
Maybe:
0 = OK
1 = Error, raise exception
2 = Error, but ok to proceed with upload
Another PR (next to #179)
This change allows for providing a postprocess binary/script, to execute custom logic post downloading a video.
In my specific case, after downloading motion detection, I want to run a script to detect if there are animals in the video (wildlife). If not: discard. If animals: upload to Youtube (wildlife channel).
Added readme, etc. etc. & relatively verbose logging of stdout and stderr if the postprocessing binary path is specified.
P.S. reason for me adding this:
https://github.com/WietseWind/Unifi-Wildlife-Detection