-
Notifications
You must be signed in to change notification settings - Fork 140
[Review][C] Export the ability to get/set the log level to the C API #1375
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
Conversation
|
/ok to test 7e7d49b |
|
Do we need this in 25.10? How do we control the log level without it? |
|
/ok to test e91a6dc |
|
/ok to test 1959150 |
@cjnolet, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/ |
|
Unfortunately now that the code is frozen we have to push it forward, so it'll need to be in 25.12. |
39abe2c to
88316a4
Compare
|
@mythrocks @benfred I have rebased this onto branch-25.12 |
|
/ok to test 0750e1c |
|
Pardon me, but I don't think the changes are complete: We'll need to change the Java build script, and change the |
|
I have that change in #1376, along with the Java changes, but it seems the way we use jextract breaks. I'll move the change to the scripts to this PR. |
|
@mythrocks In general, I find a bit odd that when you change the C api you need to update something related to the Java API. I understand the mechanics of it (jextract process a C header which includes headers from the C API, so in turn changing a C API header may have effects), but it's counterintuitive. Imagine if I was a developer purely focusing on the C side of things, with no knowledge of jextract etc. I think we need a more robust fix for this; I think jextract should use the same include directories as the C compiler. No idea how to achieve that though, ATM. |
I agree in principle. But given that these are disparate build systems, I'm not sure how we'd unify these. :/ |
|
/ok to test 97306fe |
|
/merge |
Java API for getting/setting the CuVS/raft logging Depends on #1375 Authors: - Lorenzo Dematté (https://github.com/ldematte) - Ben Frederickson (https://github.com/benfred) Approvers: - Corey J. Nolet (https://github.com/cjnolet) - Ben Frederickson (https://github.com/benfred) - MithunR (https://github.com/mythrocks) URL: #1376
CuVS uses raft underneath, which in turns uses
rapids_logger. Users of the C++ API can access directly the logger used by raft viaraft::default_logger(), and use it to get and set the log level.This is not possible for users of the C API (and of any API based on it, like the C API).
This PR adds specific C functions to close this gap.
Closes #1310