-
Notifications
You must be signed in to change notification settings - Fork 3
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
Problem: processing workflow has separate code paths for filesystem and minio watchers #867
Comments
This is complicated by the fact that the current |
djjuhasz
added a commit
that referenced
this issue
Feb 23, 2024
Fixes #867 - Update existing watcher Download() method and interface signatures to take a destination filepath (string) instead of an io.Writer stream - Add a `filewatcher.Download()` method - Create a `minio.Download()` method and move implementation specific code from `serviceImpl.Download()` to it - Update mocks
djjuhasz
added a commit
that referenced
this issue
Feb 23, 2024
Refs #867 - Update existing watcher Download() method and interface signatures to take a destination filepath (string) instead of an io.Writer stream - Add a `filewatcher.Download()` method - Create a `minio.Download()` method and move implementation specific code from `serviceImpl.Download()` to it - Update mocks
djjuhasz
added a commit
that referenced
this issue
Feb 23, 2024
Refs #867 - Update existing watcher Download() method and interface signatures to take a destination filepath (string) instead of an io.Writer stream - Add a `filewatcher.Download()` method - Create a `minio.Download()` method and move implementation specific code from `serviceImpl.Download()` to it - Update mocks
djjuhasz
added a commit
that referenced
this issue
Feb 28, 2024
Refs #867 - Update existing watcher Download() method and interface signatures to take a destination filepath (string) instead of an io.Writer stream - Add a `filewatcher.Download()` method - Create a `minio.Download()` method and move implementation specific code from `serviceImpl.Download()` to it - Update mocks
djjuhasz
added a commit
that referenced
this issue
Feb 29, 2024
Refs #867 - Update existing watcher Download() method and interface signatures to take a destination filepath (string) instead of an io.Writer stream - Add a `filewatcher.Download()` method - Create a `minio.Download()` method and move implementation specific code from `serviceImpl.Download()` to it - Update mocks
djjuhasz
added a commit
that referenced
this issue
Feb 29, 2024
Refs #867 - Update existing watcher Download() method and interface signatures to take a destination filepath (string) instead of an io.Writer stream - Add a `filewatcher.Download()` method - Create a `minio.Download()` method and move implementation specific code from `serviceImpl.Download()` to it - Update mocks
djjuhasz
added a commit
that referenced
this issue
Mar 1, 2024
Refs #867 - Update the existing watcher `Download()` method and interface signatures to take a destination filepath (string) instead of an `io.Writer` stream - Add a `filewatcher.Download()` method - Create a `minio.Download()` method and move implementation specific code from `serviceImpl.Download()` to it - Update mocks
djjuhasz
added a commit
that referenced
this issue
Mar 1, 2024
Refs #867 - Update the existing watcher `Download()` method and interface signatures to take a destination filepath (string) instead of an `io.Writer` stream - Add a `filewatcher.Download()` method - Create a `minio.Download()` method and move implementation specific code from `serviceImpl.Download()` to it - Add `MinioConfig.URL` field to set bucket config by URL for testing - Add unit tests - Update mocks
djjuhasz
added a commit
that referenced
this issue
Mar 1, 2024
Refs #867 - Update the existing watcher `Download()` method and interface signatures to take a destination filepath (string) instead of an `io.Writer` stream - Add a `filewatcher.Download()` method - Create a `minio.Download()` method and move implementation specific code from `serviceImpl.Download()` to it - Add `MinioConfig.URL` field to set bucket config by URL for testing - Add unit tests - Update mocks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the problem
The processing workflow and bundle activity use conditional branches to manage differences between the filesystem watcher and minio watcher implementations. The branching logic is hard to understand, requires some code duplication and makes the assumption that only a filesystem watcher can provide a "blob" that is a directory.
Possible solutions
The differences between the two watchers could be managed by moving the difference in implementation for copying the blob data to a local "processing" directory into their individual implementations instead of requiring branching logic (see "Avoiding conditionals by obeying the Liskov Substitution Principle" - https://sandimetz.com/99bottles).
Additional context
tinfo.req.IsDir
is true (which is only possible for a filesystem watcher) or false (possible for both minio and filesystem watchers)param.IsDir
(a local copy oftinfo.req.IsDir
) is true. This branch requires duplicate code for creating a temporary dir in the transfer dir, and stripping a top-level directoryThe text was updated successfully, but these errors were encountered: