feat(rust/driver_manager): Decouple driver search logic from the driver objects themselves#3930
Merged
Merged
Conversation
12e322f to
edfdcd4
Compare
Contributor
Author
|
@zeroshade I have more to do here, but any early feedback on the approach is appreciated. If the overall diff ends up confusing, you can see it commit by commit. This refactoring allowed me to find an UX issue: if we fail to parse a manifest, we might swallow the error and succeed because we later find a library somewhere instead of stopping early and rendering the error message about the manifest parsing. I will fix it in this PR. |
3a594aa to
55cf9c2
Compare
This commits introduce a [desirable] behavior change. Previously, if we loaded the library successfully but failed after it for some reason (e.g. error calling the driver init function), the search WOULD CONTINUE and the error would be ignored until we find another library or, more likely, return the driver not found error message. This is bad, it would be better to return the init function error. There is more refinement to do here, but I will leave it for another PR.
55cf9c2 to
7bac246
Compare
zeroshade
approved these changes
Feb 11, 2026
Member
zeroshade
left a comment
There was a problem hiding this comment.
I don't see any obvious issues here and as far as I can tell the logic is still preserved. So given that the current tests all pass this LGTM.
Thanks @felipecrv for this refactor, it's definitely helpful for this logic to be more reusable!
felipecrv
added a commit
to felipecrv/arrow-adbc
that referenced
this pull request
Feb 12, 2026
…er objects themselves (apache#3930) This is achieved through a new private sub-module called `search.rs`. In later PRs we might consider publishing some of these APIs.
felipecrv
added a commit
to dbt-labs/arrow-adbc
that referenced
this pull request
Feb 12, 2026
…er objects themselves (apache#3930) This is achieved through a new private sub-module called `search.rs`. In later PRs we might consider publishing some of these APIs.
felipecrv
added a commit
to dbt-labs/arrow-adbc
that referenced
this pull request
Feb 12, 2026
…er objects themselves (apache#3930) This is achieved through a new private sub-module called `search.rs`. In later PRs we might consider publishing some of these APIs.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
This is achieved through a new private sub-module called
search.rs. In later PRs we might consider publishing some of the APIs.