You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "silver-ymz (via GitHub)" <gi...@apache.org> on 2023/07/24 09:07:57 UTC
[GitHub] [incubator-opendal] silver-ymz opened a new pull request, #2705: feat(bindings/haskell): support layers
silver-ymz opened a new pull request, #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705
Update:
- export `ConcurrentLimitLayer`, `ImmutableIndexLayer`
- change invalid error message in `lib.rs`
About `LoggingLayer`, I think we can integrate it with haskell native log framework, like `monad-logger`. I'd like to finish it in the next PR.
--
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
[GitHub] [incubator-opendal] silver-ymz commented on pull request #2705: feat(bindings/haskell): support logging layer
Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#issuecomment-1663751555
I integrate [co-log](https://github.com/co-log/co-log) library into logging layer. Now users only need to fill `ocLogAction` with their log action. I think now users can pick and setup logging system easily and flexibly.
For the simple case of output to stdout, they can use `newOperator "memory" {ocLogAction = Just simpleMessageAction}`. For more complex case, thet can use co-log feature.
--
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
[GitHub] [incubator-opendal] silver-ymz commented on a diff in pull request #2705: feat(bindings/haskell): support layers
Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on code in PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#discussion_r1272013933
##########
bindings/haskell/src/layers.rs:
##########
@@ -0,0 +1,74 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::ffi::c_char;
+
+use ::opendal as od;
+
+#[repr(C)]
+pub struct Layer {
+ tag: LayerTag,
+ data: LayerData,
+}
+
+#[allow(dead_code)]
+#[repr(C)]
+pub enum LayerTag {
Review Comment:
Because we expose all operations as typeclass in haskell, users can complete functions similar to `layer` according to writing their own `OpMonad`. Maybe what we can do is to expose some layers writing by rust.
--
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
[GitHub] [incubator-opendal] silver-ymz commented on pull request #2705: feat(bindings/haskell): support logging layer
Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#issuecomment-1659876418
I have already complete exposing logging layer. Because haskell doesn't have unified logging framework, I design the API to pass a callback function from Haskell for rust to pass log information.
```
newOpWithLogger :: String -> HashMap String String -> LogLevel -> (String -> IO ()) -> IO (Either OpenDALError Operator)
```
--
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
[GitHub] [incubator-opendal] silver-ymz commented on a diff in pull request #2705: feat(bindings/haskell): support logging layer
Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on code in PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#discussion_r1282601556
##########
bindings/haskell/haskell-src/OpenDAL.hs:
##########
@@ -298,6 +312,30 @@ newOp scheme hashMap = do
errMsg <- peekCString (errorMessage ffiResult)
return $ Left $ OpenDALError code errMsg
+-- | Creates a new OpenDAL operator via `HashMap` with logger. The logger parameter is a function that accepts a string. OpenDAL will call this function to output logs.
+newOpWithLogger :: String -> HashMap String String -> LogLevel -> (String -> IO ()) -> IO (Either OpenDALError Operator)
Review Comment:
How about the API in new commit? I move all config about creating operator into `OperatorConfig`, and instance `IsString` for it.
For default config, users only need to write `newOperator "memory"`. For advanced config, they can use haskell feature to extend default config, like
`newOperator "memory" {ocLogConfig = Just $ OperatorLogConfig Debug logFn}`
As result, there is only `newOperator` using to create operator. If we want to add more feature about it, we just need to modify `OperatorConfig` and it won't make break change or add other APIs.
--
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
[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2705: feat(bindings/haskell): support layers
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#discussion_r1272026170
##########
bindings/haskell/src/layers.rs:
##########
@@ -0,0 +1,74 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::ffi::c_char;
+
+use ::opendal as od;
+
+#[repr(C)]
+pub struct Layer {
+ tag: LayerTag,
+ data: LayerData,
+}
+
+#[allow(dead_code)]
+#[repr(C)]
+pub enum LayerTag {
Review Comment:
> Maybe what we can do is to expose some layers writing by rust.
I think we can enable `retry` layer by default. And we don't need to provide API for users to configure layers in rust core for now. I feel like the `FFILayer` is not the way we wanna to go.
--
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
[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2705: feat(bindings/haskell): support logging layer
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#discussion_r1280359609
##########
bindings/haskell/src/logger.rs:
##########
@@ -0,0 +1,36 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::ffi::{c_char, CString};
+
+pub struct HsLogger {
+ pub callback: extern "C" fn(*const c_char),
+}
+
+impl log::Log for HsLogger {
Review Comment:
> My understanding is that `OpMonad` in haskell side provides almost same function with `HaskellLayer`. As result, it's not necessary to provide a `HaskellLayer`. Users can implement whatever they want through `OpMonad`.
Ok, seems we don't need to extra work. Maybe we just need an example for haskell users that how can they integrate their logging system with opendal?
> Users might need to integrate OpenDAL logging with their own logging system.
I don't get it. It exactly what I want: users can use their own logging system.
##########
bindings/haskell/src/logger.rs:
##########
@@ -0,0 +1,36 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::ffi::{c_char, CString};
+
+pub struct HsLogger {
+ pub callback: extern "C" fn(*const c_char),
+}
+
+impl log::Log for HsLogger {
Review Comment:
> My understanding is that `OpMonad` in haskell side provides almost same function with `HaskellLayer`. As result, it's not necessary to provide a `HaskellLayer`. Users can implement whatever they want through `OpMonad`.
Ok, seems we don't need to extra work. Maybe we just need an example for haskell users that how can they integrate their logging system with opendal?
> Users might need to integrate OpenDAL logging with their own logging system.
I don't get it. It's exactly what I want: users can use their own logging system.
--
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
[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2705: feat(bindings/haskell): support layers
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#discussion_r1271975788
##########
bindings/haskell/src/layers.rs:
##########
@@ -0,0 +1,74 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::ffi::c_char;
+
+use ::opendal as od;
+
+#[repr(C)]
+pub struct Layer {
+ tag: LayerTag,
+ data: LayerData,
+}
+
+#[allow(dead_code)]
+#[repr(C)]
+pub enum LayerTag {
Review Comment:
I don't want to expose layers to other language bindings this way. In fact, hashkell need to implement bindings in hashkell instead, like I proposed in https://github.com/apache/incubator-opendal/discussions/1960.
--
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
[GitHub] [incubator-opendal] silver-ymz commented on a diff in pull request #2705: feat(bindings/haskell): support logging layer
Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on code in PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#discussion_r1280353388
##########
bindings/haskell/src/logger.rs:
##########
@@ -0,0 +1,36 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::ffi::{c_char, CString};
+
+pub struct HsLogger {
+ pub callback: extern "C" fn(*const c_char),
+}
+
+impl log::Log for HsLogger {
Review Comment:
> I don't like the idea of exposing a HsLoggerin hashell binding directly.
If you don't like exposing a `HsLogger`, we can also implement `logging` with pure haskell without any rust code. My purpose of using `HsLogger` is to reduce duplicate code and maintain consistency between rust logs and Haskell logs.
> My current understanding is:
> - Provide a HaskellLayer which implements opendal::Layer in rust.
> - HaskellLayer should expose FFI to haskell like a trait in haskell so other libs can implement it (I don't is it a thing in haskell).
> - And finnally, users can implement whatever they want in haskell.
My understanding is that `OpMonad` in haskell side provides almost same function with `HaskellLayer`. As result, it's not necessary to provide a `HaskellLayer`. Users can implement whatever they want through `OpMonad`.
> So: haskell binding itself should know nothing about logging
Users might need to integrate OpenDAL logging with their own logging system. I don't think it's a good idea.
--
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
[GitHub] [incubator-opendal] Xuanwo commented on pull request #2705: feat(bindings/haskell): support layers
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#issuecomment-1647533676
I believe that the first layer to be implemented should be `logging`, as there are currently no users for Haskell binding. Therefore, there is no need for additional layers at this time.
--
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
[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2705: feat(bindings/haskell): support logging layer
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#discussion_r1280319859
##########
bin/oli/README.md:
##########
@@ -0,0 +1,89 @@
+# oli
Review Comment:
Those changes should not be included in this PR, maybe you need a clean rebase?
--
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
[GitHub] [incubator-opendal] silver-ymz commented on a diff in pull request #2705: feat(bindings/haskell): support logging layer
Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on code in PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#discussion_r1280393812
##########
bindings/haskell/src/logger.rs:
##########
@@ -0,0 +1,36 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::ffi::{c_char, CString};
+
+pub struct HsLogger {
+ pub callback: extern "C" fn(*const c_char),
+}
+
+impl log::Log for HsLogger {
Review Comment:
> I don't get it. It's exactly what I want: users can use their own logging system.
When researching haskell logging system, I found that hakell logging ecosystem is more diverse, and there isn't a universally-adopted standard like the log crate in rust. As result, I design simplest callback function logging system which does not rely on any existing logging framework. We can add some examples for popular logging system like `hslogger`, `katip`.
--
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
[GitHub] [incubator-opendal] silver-ymz commented on a diff in pull request #2705: feat(bindings/haskell): support layers
Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on code in PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#discussion_r1272013933
##########
bindings/haskell/src/layers.rs:
##########
@@ -0,0 +1,74 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::ffi::c_char;
+
+use ::opendal as od;
+
+#[repr(C)]
+pub struct Layer {
+ tag: LayerTag,
+ data: LayerData,
+}
+
+#[allow(dead_code)]
+#[repr(C)]
+pub enum LayerTag {
Review Comment:
Because we expose all operations as typeclass in haskell, users can complete functions similar to `layer` according to writing their own `OpMonad`. For example, previous haskell API can implement `ImmutableIndexLayer` in haskell side.
Maybe what we can do is to expose some layers writing by rust.
--
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
[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2705: feat(bindings/haskell): support logging layer
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#discussion_r1280359609
##########
bindings/haskell/src/logger.rs:
##########
@@ -0,0 +1,36 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::ffi::{c_char, CString};
+
+pub struct HsLogger {
+ pub callback: extern "C" fn(*const c_char),
+}
+
+impl log::Log for HsLogger {
Review Comment:
> My understanding is that `OpMonad` in haskell side provides almost same function with `HaskellLayer`. As result, it's not necessary to provide a `HaskellLayer`. Users can implement whatever they want through `OpMonad`.
Ok, seems we don't need to extra work. Maybe we just need an example for haskell users that how can they integrate their logging system with opendal.
##########
bindings/haskell/src/logger.rs:
##########
@@ -0,0 +1,36 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::ffi::{c_char, CString};
+
+pub struct HsLogger {
+ pub callback: extern "C" fn(*const c_char),
+}
+
+impl log::Log for HsLogger {
Review Comment:
> My understanding is that `OpMonad` in haskell side provides almost same function with `HaskellLayer`. As result, it's not necessary to provide a `HaskellLayer`. Users can implement whatever they want through `OpMonad`.
Ok, seems we don't need to extra work. Maybe we just need an example for haskell users that how can they integrate their logging system with opendal?
--
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
[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2705: feat(bindings/haskell): support logging layer
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#discussion_r1280319859
##########
bin/oli/README.md:
##########
@@ -0,0 +1,89 @@
+# oli
Review Comment:
Those changes should not be included in this PR, maybe you need a rebase?
##########
bindings/haskell/src/logger.rs:
##########
@@ -0,0 +1,36 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::ffi::{c_char, CString};
+
+pub struct HsLogger {
+ pub callback: extern "C" fn(*const c_char),
+}
+
+impl log::Log for HsLogger {
Review Comment:
I don't like the idea of exposing a `HsLogger`in hashell binding directly. My current understanding is:
- Provide a `HaskellLayer` which implements `opendal::Layer` in rust.
- `HaskellLayer` should expose FFI to haskell like a trait in haskell so other libs can implement it (I don't is it a thing in haskell).
- And finnally, users can implement whatever they want in haskell.
So: haskell binding itself should know nothing about logging
--
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
[GitHub] [incubator-opendal] silver-ymz commented on a diff in pull request #2705: feat(bindings/haskell): support logging layer
Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on code in PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#discussion_r1280363610
##########
bin/oli/README.md:
##########
@@ -0,0 +1,89 @@
+# oli
Review Comment:
fixed in new commit
--
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
[GitHub] [incubator-opendal] silver-ymz commented on pull request #2705: feat(bindings/haskell): support logging layer
Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#issuecomment-1663820121
It seems that co-log only supports ghc-9.2. For higher ghc version, we need to wait https://github.com/co-log/co-log/issues/251
--
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
[GitHub] [incubator-opendal] Xuanwo commented on pull request #2705: feat(bindings/haskell): support logging layer
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#issuecomment-1659899201
I'd like to schedule a brief meeting with you regarding this PR, if you're available. Our goal is to address any blockers that may be hindering progress.
--
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
[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2705: feat(bindings/haskell): support layers
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#discussion_r1271978284
##########
bindings/haskell/src/layers.rs:
##########
@@ -0,0 +1,74 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::ffi::c_char;
+
+use ::opendal as od;
+
+#[repr(C)]
+pub struct Layer {
+ tag: LayerTag,
+ data: LayerData,
+}
+
+#[allow(dead_code)]
+#[repr(C)]
+pub enum LayerTag {
Review Comment:
We need to implement a HaskellLayer which implements `opendal::Layer`, and then, HaskellLayer should expose as a haskell API which allows user to implement in haskell.
--
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
[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2705: feat(bindings/haskell): support layers
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#discussion_r1272026170
##########
bindings/haskell/src/layers.rs:
##########
@@ -0,0 +1,74 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::ffi::c_char;
+
+use ::opendal as od;
+
+#[repr(C)]
+pub struct Layer {
+ tag: LayerTag,
+ data: LayerData,
+}
+
+#[allow(dead_code)]
+#[repr(C)]
+pub enum LayerTag {
Review Comment:
> Maybe what we can do is to expose some layers writing by rust.
I think we can enable `retry` layer by default. And we don't need to provide API for users to configure layers in rust core for now.
--
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
[GitHub] [incubator-opendal] Xuanwo merged pull request #2705: feat(bindings/haskell): support logging layer
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705
--
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
[GitHub] [incubator-opendal] silver-ymz commented on pull request #2705: feat(bindings/haskell): support logging layer
Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#issuecomment-1663257532
Original code format was modified by myself. Now I change it to ormolu default format. I think it's better for others to make contribution.
--
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
[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2705: feat(bindings/haskell): support logging layer
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2705:
URL: https://github.com/apache/incubator-opendal/pull/2705#discussion_r1280450719
##########
bindings/haskell/src/logger.rs:
##########
@@ -0,0 +1,36 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::ffi::{c_char, CString};
+
+pub struct HsLogger {
+ pub callback: extern "C" fn(*const c_char),
+}
+
+impl log::Log for HsLogger {
Review Comment:
> As result, I design simplest callback function logging system which does not rely on any existing logging framework.
I don't think we need this. Users should pick and setup logging system on their needs.
--
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