huage1994 commented on PR #4308: URL: https://github.com/apache/zeppelin/pull/4308#issuecomment-1100849112
> @huage1994 Sorry for the late review but I wondered why you introduced `RemoteStorageOpertor`. It looks natural to make an interface and its implementation in Java but I feel like it's only for the test object of `MockRemoteStorageOperator` If it does, how about using mockito instead of making MockRemoteOperator? Rest of them, I'm reading and will finish reviewing them by tomorrow. Hi @jongyoul , thanks a lot for your reviews! `MockRemoteStorageOperator` is a mock with the local file system to do real test for OSSNotebookRepo. I think it is more sufficient than using mockito and it did help me find some problem when running Github CI task. Additionally I introduce `RemoteStorageOpertor` because I think the code in `NotebookRepo` should avoid underlying complex logic of storage operation. By the way I think `RemoteStorageOpertor` and `MockRemoteStorageOperator` can be also reused by other Notebook Storage in the future. Ideally, we only need to implement `RemoteStorageOpertor` instead of NotebookRepo. -- 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: dev-unsubscr...@zeppelin.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
