Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(bindings/haskell): support logging layer #2705

Merged
merged 13 commits into from Aug 4, 2023
Merged

Conversation

silver-ymz
Copy link
Member

@silver-ymz silver-ymz commented Jul 24, 2023

Update:

  • add logging layer
  • change invalid error message in lib.rs

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.

@Xuanwo
Copy link
Member

Xuanwo commented Jul 24, 2023

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.

@silver-ymz silver-ymz marked this pull request as draft July 24, 2023 10:27
@silver-ymz silver-ymz changed the title feat(bindings/haskell): support layers feat(bindings/haskell): support logging layer Jul 24, 2023
…xLayer

Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
@silver-ymz silver-ymz marked this pull request as ready for review August 1, 2023 08:55
@silver-ymz silver-ymz requested a review from PsiACE as a code owner August 1, 2023 08:55
@silver-ymz
Copy link
Member Author

silver-ymz commented Aug 1, 2023

I have already completed 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)

I make some mistakes about git rebase. There are some strange things happened about git.

@silver-ymz silver-ymz requested a review from Xuanwo August 1, 2023 08:57
bin/oli/README.md Outdated Show resolved Hide resolved
bindings/haskell/src/logger.rs Show resolved Hide resolved
@Xuanwo
Copy link
Member

Xuanwo commented Aug 1, 2023

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.

Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
@silver-ymz
Copy link
Member Author

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.

Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
@silver-ymz
Copy link
Member Author

I integrate 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.

Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
@silver-ymz
Copy link
Member Author

It seems that co-log only supports ghc-9.2. For higher ghc version, we need to wait co-log/co-log#251

Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No better ideas, let's move!

@Xuanwo Xuanwo merged commit 718d65f into main Aug 4, 2023
20 checks passed
@Xuanwo Xuanwo deleted the feat/haskell-layers branch August 4, 2023 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants