liucao-dd commented on PR #206: URL: https://github.com/apache/cassandra-analytics/pull/206#issuecomment-4435769809
> Thanks for the patch, this is a great addition! I will take a closer look, but a few high level questions: > > * Did you consider putting this in a separate module (e.g. `cassandra-analytics-s3`). The Sidecar DataLayer should never had been put in `cassandra-analytics-core` and should eventually be moved to its own module. wasn't aware that we wanted the sidecar data layer to live outside of core. happy to discuss the appropriate modularization. A s3 specific one could work, or we can have a batch read specific module? the s3 implementation inherits a good amount of work from the sidecar one. > * I suppose every user might have a slightly different backup path format, is this made pluggable by the `BackupReader` interface? yes, i didn't include the internal concrete implementation of this interface on this PR because it is tied to a vendor we use. But the goal is to make it pluggable per needs of different organization, using the `BackupReaderRegistry.register()` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
