feat(rust/core): add function to load driver manifests #3099
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zeroshade
commented
Jul 3, 2025
Member
Author
zeroshade
left a comment
zeroshade
left a comment
There was a problem hiding this comment.
Come one, come all, roast my Rust code and tell me how to make it better.... please!
Member
Author
lidavidm
reviewed
Jul 4, 2025
mbrobbel
reviewed
Jul 4, 2025
paleolimbot
approved these changes
Jul 4, 2025
Member
paleolimbot
left a comment
paleolimbot
left a comment
There was a problem hiding this comment.
Thank you for this! Only optional improvements from me 🙂
felipecrv
reviewed
Jul 4, 2025
Co-authored-by: David Li <[email protected]>
Member
Author
|
I've also updated this to have the same windows registry logic that the C++ adbc_driver_manager has. |
amoeba
reviewed
Jul 8, 2025
Co-authored-by: Bryce Mecum <[email protected]>
paleolimbot
approved these changes
Jul 9, 2025
lidavidm
approved these changes
Jul 9, 2025
eitsupi
reviewed
Jul 9, 2025
mbrobbel
reviewed
Jul 9, 2025
Co-authored-by: Matthijs Brobbel <[email protected]>
Co-authored-by: eitsupi <[email protected]>
mbrobbel
reviewed
Jul 9, 2025
eitsupi
approved these changes
Jul 10, 2025
Contributor
eitsupi
left a comment
eitsupi
left a comment
There was a problem hiding this comment.
Looking forward to seeing this working!
Some minor comments.
Co-authored-by: Matthijs Brobbel <[email protected]>
Co-authored-by: eitsupi <[email protected]>
Member
Author
|
@mbrobbel any further comments here? Or are we looking good? 😄 |
mbrobbel
approved these changes
Jul 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following the pattern of the C++ driver manager and the various bindings to it, we also have to add support for the Rust driver manager to handle and load drivers via manifest files too. Since the Rust driver manager doesn't bind to the C++ one like everyone else, we need to add the support in rust directly to locate, open, and parse driver manifests.
This PR adds a new function
ManagedDriver::find_load_from_namealong with constants for theLoadFlagsto control what paths are searched. The basic logic mirrors the C++ implementation. Unit tests are also included.To control the tests, two new features are added:
driver_manager_test_libwhich expects an env varADBC_DRIVER_MANAGER_TEST_LIBto be set, just like the C++ driver manager tests, to the path for an adbc library to load. Anddriver_manager_test_manifest_userwhich controls whether the test that interacts with the user's config directory should be run or not.