GitHub user koushiro created a discussion: Unify code pattern of service implementation
## Background After completing the [migration of adapter-based services](https://github.com/apache/opendal/issues/5739) and [splitting the code of each service into a core + backend pattern](https://github.com/apache/opendal/issues/5702), the current services now have a relatively consistent implementation pattern and can more easily implement corresponding operations according to their own characteristics. However, there are still inconsistencies in implementation patterns across various services. This discussion aims to discuss and unify these coding details to improve code consistency and maintainability. ## Proposed Changes ### 1. Debug Trait Implementation for Service Builders **Question**: Should we implement the `Debug` trait for all service builders? Currently, some service builders implement `Debug` while others don't. We should establish a consistent convention across all services. As far as I know, there is currently no trait bound requiring the service builder to implement Debug. Perhaps downstream developers might need to debug specific configurations within the service builder? **Action Items**: - [ ] Audit all service builders to identify which ones implement `Debug` - [ ] Decide whether `Debug` should be required for all builders - [ ] Implement or document the decision ### 2. Module and Structure Renaming **Proposed Renames**: - `ServiceAccessor` → `ServiceBackend` (to align the naming conventions of `backend` module) - `delete` module → `deleter` module (to align with naming conventions like `writer`, `reader`, `lister`) **Action Items**: - [ ] Rename `ServiceAccessor` to `ServiceBackend` across all services - [ ] Rename `delete` modules to `deleter` for consistency - [ ] Update documentation and examples ### 3. Remove Empty `list` Operation Implementations **Issue**: Currently, some services include an empty `list` implementation, as shown below. I believe that if list operations are not supported or have not yet been implemented, it is sufficient to simply omit the implementation. ```rust async fn list(&self, path: &str, _: OpList) -> Result<(RpList, Self::Lister)> { let _ = build_abs_path(&self.root, path); Ok((RpList::default(), ())) } ``` **Proposed Solution**: - Only keep list implementations in services that actually provide this functionality - Remove useless `list` implementations from services that don't support this operation or have not implemented it yet. **Action Items**: - [ ] Identify all empty list implementations - [ ] Audit which services should support list operations - [ ] Remove unnecessary empty implementations - [ ] Document which services support which operations ### 4. Unify Scheme Usage **Current Issue**: Multiple approaches to specifying schemes across services. **Proposed Solution**: - Prefer direct string-based scheme specification over enum-based approach - Consider removing the `Scheme` enum in favor of string literals **Action Items**: - [ ] Audit current scheme usage across all services - [ ] Evaluate the cost-benefit of removing `Scheme` enum ## Discussion Points - Should we do these changes in a single PR or break them into multiple focused PRs? - What's the priority order for these changes? - Are there any services that should be excluded from these standardizations? - Should we create a service implementation guide/template after these changes? ## Related Issues/PRs - https://github.com/apache/opendal/issues/5739 - https://github.com/apache/opendal/issues/5702 ## Help Wanted Feedback on: - Whether these changes align with the project's architectural goals - Any additional consistency issues that should be addressed - Implementation approach and timeline GitHub link: https://github.com/apache/opendal/discussions/6752 ---- This is an automatically sent email for [email protected]. To unsubscribe, please send an email to: [email protected]
