Re: [PR] feat: add additional layers support [opendal]

["Retrospection (via GitHub)" <gi...@apache.org>]

Retrospection commented on code in PR #7128:
URL: https://github.com/apache/opendal/pull/7128#discussion_r3104396868


##########
bindings/java/src/main/java/org/apache/opendal/layer/LoggingLayer.java:
##########
@@ -24,6 +24,9 @@
 /**
  * This layer adds structured logging for every operation.
  *
+ * <p>Note: This layer requires proper logging setup (e.g., log4j2, java.util.logging) to work.

Review Comment:
   > Hi, please understand your changes first. `LoggingLayer` integrate rust's `log` crate and thus requires we to provide a `set_logger` call. Users can't configure that through `log4j2` or `java.util.logging`.
   > 
   > Maybe we can start a discussion thread on how we do that.
   
   I have been studying the abstraction role of the OpenDAL layer and now understand the issues with the Java LoggingLayer implementation in my previous PR.
   
   I would like to ask:
   1. Is it okay if I temporarily abandon the implementation of the LoggingLayer part for now?
   2. Alternatively, should I pursue a deeper integration with SLF4J, Log4j, or java.util.logging instead?



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org