feat: load driver from uri#3694
Conversation
There was a problem hiding this comment.
If we're going to do this, the logic to detect URI/driver from the driver arg needs to be either removed or made the same (as it doesn't split on colon). (Also I think that logic should probably validate that the "scheme" is alphanumeric+a few other symbols instead of accepting anything)
There was a problem hiding this comment.
do we really need to validate the scheme here rather than just let the driver handle validating the URI itself?
There was a problem hiding this comment.
I'm not saying to validate the scheme; just avoid a false positive if there's something that looks like a colon slash later in the "uri" but the part before it wouldn't have been a valid scheme in the first place
5a014f8 to
0553228
Compare
|
I have no idea why python is failing to find the bigquery driver shared lib on windows in the CI. Any ideas @lidavidm? |
|
Could you share the CI failure log? |
|
https://github.com/apache/arrow-adbc/actions/runs/19443805171/job/55633450295#step:12:181 |
|
@kou turned out that the issue was me badly handling windows file paths. Though I haven't been able to figure out the cause of this failure: https://github.com/apache/arrow-adbc/actions/runs/19449897622/job/55652709570?pr=3694 and the almalinux-9 failure (https://github.com/apache/arrow-adbc/actions/runs/19449897615/job/55652412064?pr=3694) |
|
Great! I'll take a look at them! AlmaLinux 9: Installing old Apache Arrow from EPEL isn't expected. I'll try why it happens on local. |
|
Thanks!! |
|
AlmaLinux 9: I think that https://lists.apache.org/thread/rqsvtdj2jxks03z8wzymk46br71dqd4h is the same problem. Windows: This has been failed since 2964c54 but the change will not be related. Some CI environments may be changed (e.g. package versions in Conda): The first failure log: https://github.com/apache/arrow-adbc/actions/runs/18672884467/job/53237442892 Anyway, this is also not related to this PR. |
Co-authored-by: David Li <li.davidm96@gmail.com>
|
@kou Thanks for looking into this, do you think we could create a separate PR to fix the almalinux issue at least? |
|
Yes. But I don't have a correct fix for the AlmaLinux 9 failure for now. (I think that the patch in the mailing list post is a workaround.) So could you create a separated issue for now? |
|
Thanks! |
Allow loading a driver via the driver manager using only a URI