felipecrv commented on code in PR #3973:
URL: https://github.com/apache/arrow-adbc/pull/3973#discussion_r2844047899
##########
rust/driver_manager/src/lib.rs:
##########
@@ -379,20 +491,118 @@ impl ManagedDatabase {
additional_search_paths: Option<Vec<PathBuf>>,
opts: impl IntoIterator<Item = (<Self as Optionable>::Option,
OptionValue)>,
) -> Result<Self> {
- let (driver, final_uri) = parse_driver_uri(uri)?;
-
- let mut drv = ManagedDriver::load_from_name(
- driver,
+ let profile_provider = FilesystemProfileProvider;
+ Self::from_uri_with_profile_provider(
+ uri,
entrypoint,
version,
load_flags,
additional_search_paths,
- )?;
+ profile_provider,
+ opts,
+ )
+ }
- drv.new_database_with_opts(opts.into_iter().chain(std::iter::once((
- OptionDatabase::Uri,
- OptionValue::String(final_uri.to_string()),
- ))))
+ /// Creates a new database connection from a URI with a custom profile
provider.
+ ///
+ /// This advanced method allows using a custom implementation of
+ /// [`ConnectionProfileProvider`] to load profiles from alternative sources
+ /// (e.g., remote configuration services, encrypted storage, etc.).
+ ///
+ /// # Arguments
+ ///
+ /// * `uri` - The connection URI or profile reference
+ /// * `entrypoint` - Optional driver entrypoint name
+ /// * `version` - ADBC version to use
+ /// * `load_flags` - Flags controlling driver loading behavior
+ /// * `additional_search_paths` - Optional paths to search for drivers or
profiles
+ /// * `profile_provider` - Custom profile provider implementation
+ /// * `opts` - Database options to apply (override profile options)
+ ///
+ /// # Returns
+ ///
+ /// A configured `ManagedDatabase` using the custom profile provider.
+ ///
+ /// # Examples
+ ///
+ /// ```no_run
+ /// use adbc_core::options::{AdbcVersion, OptionDatabase, OptionValue};
+ /// use adbc_driver_manager::ManagedDatabase;
+ /// use
adbc_driver_manager::connection_profiles::FilesystemProfileProvider;
+ /// use adbc_core::LOAD_FLAG_DEFAULT;
+ ///
+ /// let provider = FilesystemProfileProvider;
+ /// let opts = vec![(OptionDatabase::Username,
OptionValue::String("admin".to_string()))];
+ ///
+ /// let db = ManagedDatabase::from_uri_with_profile_provider(
+ /// "profile://my_database",
+ /// None,
+ /// AdbcVersion::V100,
+ /// LOAD_FLAG_DEFAULT,
+ /// None,
+ /// provider,
+ /// opts,
+ /// )?;
+ /// # Ok::<(), adbc_core::error::Error>(())
+ /// ```
+ pub fn from_uri_with_profile_provider(
+ uri: &str,
+ entrypoint: Option<&[u8]>,
+ version: AdbcVersion,
+ load_flags: LoadFlags,
+ additional_search_paths: Option<Vec<PathBuf>>,
+ profile_provider: impl ConnectionProfileProvider,
+ opts: impl IntoIterator<Item = (<Self as Optionable>::Option,
OptionValue)>,
+ ) -> Result<Self> {
+ let mut drv: ManagedDriver;
+ let result = parse_driver_uri(uri)?;
+ match result {
+ SearchResult::DriverUri(driver, final_uri) => {
+ drv = ManagedDriver::load_from_name(
+ driver,
+ entrypoint,
+ version,
+ load_flags,
+ additional_search_paths.clone(),
+ )?;
+
+
drv.new_database_with_opts(opts.into_iter().chain(std::iter::once((
+ OptionDatabase::Uri,
+ OptionValue::String(final_uri.to_string()),
+ ))))
+ }
+ SearchResult::Profile(profile) => {
+ let profile =
+ profile_provider.get_profile(profile,
additional_search_paths.clone())?;
+ let (driver_name, init_func) = profile.get_driver_name()?;
+
+ if let Some(init_fn) = init_func {
+ drv = ManagedDriver::load_static(init_fn, version)?;
+ } else {
+ drv = ManagedDriver::load_from_name(
+ driver_name,
+ entrypoint,
+ version,
+ load_flags,
+ additional_search_paths,
+ )?;
+ }
+
+ let profile_opts: Vec<(OptionDatabase, OptionValue)> = profile
+ .get_options()?
+ .into_iter()
+ .map(|(k, v)| {
Review Comment:
```suggestion
.map(|(k, v)| -> Result<(OptionDatabase, OptionValue),
Error> {
```
##########
rust/driver_manager/src/lib.rs:
##########
@@ -379,20 +491,118 @@ impl ManagedDatabase {
additional_search_paths: Option<Vec<PathBuf>>,
opts: impl IntoIterator<Item = (<Self as Optionable>::Option,
OptionValue)>,
) -> Result<Self> {
- let (driver, final_uri) = parse_driver_uri(uri)?;
-
- let mut drv = ManagedDriver::load_from_name(
- driver,
+ let profile_provider = FilesystemProfileProvider;
+ Self::from_uri_with_profile_provider(
+ uri,
entrypoint,
version,
load_flags,
additional_search_paths,
- )?;
+ profile_provider,
+ opts,
+ )
+ }
- drv.new_database_with_opts(opts.into_iter().chain(std::iter::once((
- OptionDatabase::Uri,
- OptionValue::String(final_uri.to_string()),
- ))))
+ /// Creates a new database connection from a URI with a custom profile
provider.
+ ///
+ /// This advanced method allows using a custom implementation of
+ /// [`ConnectionProfileProvider`] to load profiles from alternative sources
+ /// (e.g., remote configuration services, encrypted storage, etc.).
+ ///
+ /// # Arguments
+ ///
+ /// * `uri` - The connection URI or profile reference
+ /// * `entrypoint` - Optional driver entrypoint name
+ /// * `version` - ADBC version to use
+ /// * `load_flags` - Flags controlling driver loading behavior
+ /// * `additional_search_paths` - Optional paths to search for drivers or
profiles
+ /// * `profile_provider` - Custom profile provider implementation
+ /// * `opts` - Database options to apply (override profile options)
+ ///
+ /// # Returns
+ ///
+ /// A configured `ManagedDatabase` using the custom profile provider.
+ ///
+ /// # Examples
+ ///
+ /// ```no_run
+ /// use adbc_core::options::{AdbcVersion, OptionDatabase, OptionValue};
+ /// use adbc_driver_manager::ManagedDatabase;
+ /// use
adbc_driver_manager::connection_profiles::FilesystemProfileProvider;
+ /// use adbc_core::LOAD_FLAG_DEFAULT;
+ ///
+ /// let provider = FilesystemProfileProvider;
+ /// let opts = vec![(OptionDatabase::Username,
OptionValue::String("admin".to_string()))];
+ ///
+ /// let db = ManagedDatabase::from_uri_with_profile_provider(
+ /// "profile://my_database",
+ /// None,
+ /// AdbcVersion::V100,
+ /// LOAD_FLAG_DEFAULT,
+ /// None,
+ /// provider,
+ /// opts,
+ /// )?;
+ /// # Ok::<(), adbc_core::error::Error>(())
+ /// ```
+ pub fn from_uri_with_profile_provider(
+ uri: &str,
+ entrypoint: Option<&[u8]>,
+ version: AdbcVersion,
+ load_flags: LoadFlags,
+ additional_search_paths: Option<Vec<PathBuf>>,
+ profile_provider: impl ConnectionProfileProvider,
+ opts: impl IntoIterator<Item = (<Self as Optionable>::Option,
OptionValue)>,
+ ) -> Result<Self> {
+ let mut drv: ManagedDriver;
+ let result = parse_driver_uri(uri)?;
+ match result {
+ SearchResult::DriverUri(driver, final_uri) => {
+ drv = ManagedDriver::load_from_name(
+ driver,
+ entrypoint,
+ version,
+ load_flags,
+ additional_search_paths.clone(),
+ )?;
Review Comment:
This can become
```rust
DriverLibrary::search(name, load_flags, additional_search_paths)?
```
and you can make every branch return `SearchHit`. Then at the end you can:
```rust
let entrypoint = search_hit.resolve_entrypoint(entrypoint).to_vec();
Self::load_from_library(search_hit.library, entrypoint.as_ref(),
version)
```
Then, at the end of all this, you can extract a function that returns
`SearchHit` to `search.rs` and keep the `load_from_library` call here. This is
what enables different users of `search.rs` (which I intend to make more public
in the upstream too) to instantiate structs that are not `ManagedDriver`.
##########
rust/driver_manager/src/lib.rs:
##########
@@ -379,20 +491,118 @@ impl ManagedDatabase {
additional_search_paths: Option<Vec<PathBuf>>,
opts: impl IntoIterator<Item = (<Self as Optionable>::Option,
OptionValue)>,
) -> Result<Self> {
- let (driver, final_uri) = parse_driver_uri(uri)?;
-
- let mut drv = ManagedDriver::load_from_name(
- driver,
+ let profile_provider = FilesystemProfileProvider;
+ Self::from_uri_with_profile_provider(
+ uri,
entrypoint,
version,
load_flags,
additional_search_paths,
- )?;
+ profile_provider,
+ opts,
+ )
+ }
- drv.new_database_with_opts(opts.into_iter().chain(std::iter::once((
- OptionDatabase::Uri,
- OptionValue::String(final_uri.to_string()),
- ))))
+ /// Creates a new database connection from a URI with a custom profile
provider.
+ ///
+ /// This advanced method allows using a custom implementation of
+ /// [`ConnectionProfileProvider`] to load profiles from alternative sources
+ /// (e.g., remote configuration services, encrypted storage, etc.).
+ ///
+ /// # Arguments
+ ///
+ /// * `uri` - The connection URI or profile reference
+ /// * `entrypoint` - Optional driver entrypoint name
+ /// * `version` - ADBC version to use
+ /// * `load_flags` - Flags controlling driver loading behavior
+ /// * `additional_search_paths` - Optional paths to search for drivers or
profiles
+ /// * `profile_provider` - Custom profile provider implementation
+ /// * `opts` - Database options to apply (override profile options)
+ ///
+ /// # Returns
+ ///
+ /// A configured `ManagedDatabase` using the custom profile provider.
+ ///
+ /// # Examples
+ ///
+ /// ```no_run
+ /// use adbc_core::options::{AdbcVersion, OptionDatabase, OptionValue};
+ /// use adbc_driver_manager::ManagedDatabase;
+ /// use
adbc_driver_manager::connection_profiles::FilesystemProfileProvider;
+ /// use adbc_core::LOAD_FLAG_DEFAULT;
+ ///
+ /// let provider = FilesystemProfileProvider;
+ /// let opts = vec![(OptionDatabase::Username,
OptionValue::String("admin".to_string()))];
+ ///
+ /// let db = ManagedDatabase::from_uri_with_profile_provider(
+ /// "profile://my_database",
+ /// None,
+ /// AdbcVersion::V100,
+ /// LOAD_FLAG_DEFAULT,
+ /// None,
+ /// provider,
+ /// opts,
+ /// )?;
+ /// # Ok::<(), adbc_core::error::Error>(())
+ /// ```
+ pub fn from_uri_with_profile_provider(
+ uri: &str,
+ entrypoint: Option<&[u8]>,
+ version: AdbcVersion,
+ load_flags: LoadFlags,
+ additional_search_paths: Option<Vec<PathBuf>>,
+ profile_provider: impl ConnectionProfileProvider,
+ opts: impl IntoIterator<Item = (<Self as Optionable>::Option,
OptionValue)>,
+ ) -> Result<Self> {
+ let mut drv: ManagedDriver;
+ let result = parse_driver_uri(uri)?;
+ match result {
+ SearchResult::DriverUri(driver, final_uri) => {
+ drv = ManagedDriver::load_from_name(
+ driver,
+ entrypoint,
+ version,
+ load_flags,
+ additional_search_paths.clone(),
+ )?;
+
+
drv.new_database_with_opts(opts.into_iter().chain(std::iter::once((
+ OptionDatabase::Uri,
+ OptionValue::String(final_uri.to_string()),
+ ))))
+ }
+ SearchResult::Profile(profile) => {
+ let profile =
+ profile_provider.get_profile(profile,
additional_search_paths.clone())?;
+ let (driver_name, init_func) = profile.get_driver_name()?;
+
+ if let Some(init_fn) = init_func {
+ drv = ManagedDriver::load_static(init_fn, version)?;
+ } else {
+ drv = ManagedDriver::load_from_name(
+ driver_name,
+ entrypoint,
+ version,
+ load_flags,
+ additional_search_paths,
+ )?;
+ }
+
+ let profile_opts: Vec<(OptionDatabase, OptionValue)> = profile
+ .get_options()?
+ .into_iter()
+ .map(|(k, v)| {
+ if let OptionValue::String(s) = v {
+ let result = process_profile_value(&s)?;
+ Ok::<(OptionDatabase, OptionValue), Error>((k,
result))
Review Comment:
```suggestion
Ok((k, result))
```
##########
rust/driver_manager/src/search.rs:
##########
@@ -816,28 +817,221 @@ fn get_search_paths(lvls: LoadFlags) -> Vec<PathBuf> {
result
}
-pub(crate) fn parse_driver_uri(uri: &str) -> Result<(&str, &str)> {
+/// Locates a connection profile file on the filesystem.
+///
+/// This function searches for profile files with a `.toml` extension using the
+/// following strategy:
+///
+/// 1. If `name` is an absolute path with `.toml` extension, verify it exists
+/// 2. If `name` is an absolute path without extension, add `.toml` and return
+/// 3. If `name` is a relative path that exists in the current directory,
return it
+/// 4. Search configured profile directories in order:
+/// - Additional paths provided
+/// - `ADBC_PROFILE_PATH` environment variable paths
+/// - Conda prefix path (if built with `conda_build`)
+/// - User configuration directory
+///
+/// # Arguments
+///
+/// * `name` - Profile name or path (e.g., "my_profile",
"/path/to/profile.toml")
+/// * `additional_path_list` - Optional additional directories to search
+///
+/// # Returns
+///
+/// The absolute path to the located profile file.
+///
+/// # Errors
+///
+/// Returns `Status::NotFound` if the profile cannot be located in any search
path.
+pub(crate) fn find_filesystem_profile(
+ name: impl AsRef<str>,
+ additional_path_list: Option<Vec<PathBuf>>,
+) -> Result<PathBuf> {
+ // Convert the name to a PathBuf to ensure proper platform-specific path
handling.
+ // This normalizes forward slashes to backslashes on Windows.
+ let profile_path = PathBuf::from_slash(name.as_ref());
+ let profile_path = profile_path.as_path();
Review Comment:
You're shadowing the `profile_path: PathBuf` but then you are allocating a
new `PathBuf` from the `Path` to return it.
You should invert: use `profile_path.as_path()` where you need a `Path` and
delete the `.to_path_buf()` and use just `profile_path: PathBuf`.
##########
rust/driver_manager/src/search.rs:
##########
@@ -816,28 +817,221 @@ fn get_search_paths(lvls: LoadFlags) -> Vec<PathBuf> {
result
}
-pub(crate) fn parse_driver_uri(uri: &str) -> Result<(&str, &str)> {
+/// Locates a connection profile file on the filesystem.
+///
+/// This function searches for profile files with a `.toml` extension using the
+/// following strategy:
+///
+/// 1. If `name` is an absolute path with `.toml` extension, verify it exists
+/// 2. If `name` is an absolute path without extension, add `.toml` and return
+/// 3. If `name` is a relative path that exists in the current directory,
return it
+/// 4. Search configured profile directories in order:
+/// - Additional paths provided
+/// - `ADBC_PROFILE_PATH` environment variable paths
+/// - Conda prefix path (if built with `conda_build`)
+/// - User configuration directory
+///
+/// # Arguments
+///
+/// * `name` - Profile name or path (e.g., "my_profile",
"/path/to/profile.toml")
+/// * `additional_path_list` - Optional additional directories to search
+///
+/// # Returns
+///
+/// The absolute path to the located profile file.
+///
+/// # Errors
+///
+/// Returns `Status::NotFound` if the profile cannot be located in any search
path.
+pub(crate) fn find_filesystem_profile(
+ name: impl AsRef<str>,
+ additional_path_list: Option<Vec<PathBuf>>,
+) -> Result<PathBuf> {
+ // Convert the name to a PathBuf to ensure proper platform-specific path
handling.
+ // This normalizes forward slashes to backslashes on Windows.
+ let profile_path = PathBuf::from_slash(name.as_ref());
+ let profile_path = profile_path.as_path();
+
+ // Handle absolute paths
+ if profile_path.is_absolute() {
+ let has_toml_ext = profile_path.extension().is_some_and(|ext| ext ==
"toml");
Review Comment:
This variable can be defined right after `profile_path` is defined.
##########
rust/driver_manager/src/search.rs:
##########
@@ -816,28 +817,221 @@ fn get_search_paths(lvls: LoadFlags) -> Vec<PathBuf> {
result
}
-pub(crate) fn parse_driver_uri(uri: &str) -> Result<(&str, &str)> {
+/// Locates a connection profile file on the filesystem.
+///
+/// This function searches for profile files with a `.toml` extension using the
+/// following strategy:
+///
+/// 1. If `name` is an absolute path with `.toml` extension, verify it exists
+/// 2. If `name` is an absolute path without extension, add `.toml` and return
+/// 3. If `name` is a relative path that exists in the current directory,
return it
+/// 4. Search configured profile directories in order:
+/// - Additional paths provided
+/// - `ADBC_PROFILE_PATH` environment variable paths
+/// - Conda prefix path (if built with `conda_build`)
+/// - User configuration directory
+///
+/// # Arguments
+///
+/// * `name` - Profile name or path (e.g., "my_profile",
"/path/to/profile.toml")
+/// * `additional_path_list` - Optional additional directories to search
+///
+/// # Returns
+///
+/// The absolute path to the located profile file.
+///
+/// # Errors
+///
+/// Returns `Status::NotFound` if the profile cannot be located in any search
path.
+pub(crate) fn find_filesystem_profile(
+ name: impl AsRef<str>,
Review Comment:
`name: &str` because it's not worth it binary-size wise to specialize this
in the binary for each `AsRef<str>` implementation.
##########
rust/driver_manager/src/search.rs:
##########
@@ -816,28 +817,221 @@ fn get_search_paths(lvls: LoadFlags) -> Vec<PathBuf> {
result
}
-pub(crate) fn parse_driver_uri(uri: &str) -> Result<(&str, &str)> {
+/// Locates a connection profile file on the filesystem.
+///
+/// This function searches for profile files with a `.toml` extension using the
+/// following strategy:
+///
+/// 1. If `name` is an absolute path with `.toml` extension, verify it exists
+/// 2. If `name` is an absolute path without extension, add `.toml` and return
+/// 3. If `name` is a relative path that exists in the current directory,
return it
+/// 4. Search configured profile directories in order:
+/// - Additional paths provided
+/// - `ADBC_PROFILE_PATH` environment variable paths
+/// - Conda prefix path (if built with `conda_build`)
+/// - User configuration directory
+///
+/// # Arguments
+///
+/// * `name` - Profile name or path (e.g., "my_profile",
"/path/to/profile.toml")
+/// * `additional_path_list` - Optional additional directories to search
+///
+/// # Returns
+///
+/// The absolute path to the located profile file.
+///
+/// # Errors
+///
+/// Returns `Status::NotFound` if the profile cannot be located in any search
path.
+pub(crate) fn find_filesystem_profile(
+ name: impl AsRef<str>,
+ additional_path_list: Option<Vec<PathBuf>>,
+) -> Result<PathBuf> {
+ // Convert the name to a PathBuf to ensure proper platform-specific path
handling.
+ // This normalizes forward slashes to backslashes on Windows.
+ let profile_path = PathBuf::from_slash(name.as_ref());
+ let profile_path = profile_path.as_path();
+
+ // Handle absolute paths
+ if profile_path.is_absolute() {
+ let has_toml_ext = profile_path.extension().is_some_and(|ext| ext ==
"toml");
+
+ if has_toml_ext {
+ // Has .toml extension - verify it exists
+ return if profile_path.is_file() {
+ Ok(profile_path.to_path_buf())
+ } else {
+ Err(Error::with_message_and_status(
+ format!("Profile not found: {}", profile_path.display()),
+ Status::NotFound,
+ ))
+ };
+ }
+
+ // No .toml extension - add it
+ return Ok(profile_path.with_extension("toml"));
+ }
+
+ // For relative paths, check if it's a file at the current location first
+ if profile_path.is_file() {
+ return Ok(profile_path.to_path_buf());
+ }
+
+ // Search in the configured paths
+ let path_list = get_profile_search_paths(additional_path_list);
+ let has_toml_ext = profile_path.extension().is_some_and(|ext| ext ==
"toml");
+
+ path_list
+ .iter()
+ .find_map(|path| {
+ if has_toml_ext {
+ // Name already has .toml extension, use it as-is
+ let full_path = path.join(profile_path);
+ if full_path.is_file() {
+ return Some(full_path);
+ }
+ }
+ // Try adding .toml extension
+ let mut full_path = path.join(profile_path);
+ full_path.set_extension("toml");
+ if full_path.is_file() {
+ return Some(full_path);
+ }
+ None
Review Comment:
Not just a style change. It avoid checking the same path twice when it has
the .toml extension and doesn't exist.
```suggestion
let mut full_path = path.join(profile_path);
if has_toml_ext {
// Name already has .toml extension, use it as-is
if full_path.is_file() {
Some(full_path)
} else {
None
}
} else {
// Try adding .toml extension
full_path.set_extension("toml");
if full_path.is_file() {
Some(full_path)
} else {
None
}
}
```
##########
rust/driver_manager/src/lib.rs:
##########
@@ -379,20 +491,118 @@ impl ManagedDatabase {
additional_search_paths: Option<Vec<PathBuf>>,
opts: impl IntoIterator<Item = (<Self as Optionable>::Option,
OptionValue)>,
) -> Result<Self> {
- let (driver, final_uri) = parse_driver_uri(uri)?;
-
- let mut drv = ManagedDriver::load_from_name(
- driver,
+ let profile_provider = FilesystemProfileProvider;
+ Self::from_uri_with_profile_provider(
+ uri,
entrypoint,
version,
load_flags,
additional_search_paths,
- )?;
+ profile_provider,
+ opts,
+ )
+ }
- drv.new_database_with_opts(opts.into_iter().chain(std::iter::once((
- OptionDatabase::Uri,
- OptionValue::String(final_uri.to_string()),
- ))))
+ /// Creates a new database connection from a URI with a custom profile
provider.
+ ///
+ /// This advanced method allows using a custom implementation of
+ /// [`ConnectionProfileProvider`] to load profiles from alternative sources
+ /// (e.g., remote configuration services, encrypted storage, etc.).
+ ///
+ /// # Arguments
+ ///
+ /// * `uri` - The connection URI or profile reference
+ /// * `entrypoint` - Optional driver entrypoint name
+ /// * `version` - ADBC version to use
+ /// * `load_flags` - Flags controlling driver loading behavior
+ /// * `additional_search_paths` - Optional paths to search for drivers or
profiles
+ /// * `profile_provider` - Custom profile provider implementation
+ /// * `opts` - Database options to apply (override profile options)
+ ///
+ /// # Returns
+ ///
+ /// A configured `ManagedDatabase` using the custom profile provider.
+ ///
+ /// # Examples
+ ///
+ /// ```no_run
+ /// use adbc_core::options::{AdbcVersion, OptionDatabase, OptionValue};
+ /// use adbc_driver_manager::ManagedDatabase;
+ /// use
adbc_driver_manager::connection_profiles::FilesystemProfileProvider;
+ /// use adbc_core::LOAD_FLAG_DEFAULT;
+ ///
+ /// let provider = FilesystemProfileProvider;
+ /// let opts = vec![(OptionDatabase::Username,
OptionValue::String("admin".to_string()))];
+ ///
+ /// let db = ManagedDatabase::from_uri_with_profile_provider(
+ /// "profile://my_database",
+ /// None,
+ /// AdbcVersion::V100,
+ /// LOAD_FLAG_DEFAULT,
+ /// None,
+ /// provider,
+ /// opts,
+ /// )?;
+ /// # Ok::<(), adbc_core::error::Error>(())
+ /// ```
+ pub fn from_uri_with_profile_provider(
+ uri: &str,
+ entrypoint: Option<&[u8]>,
+ version: AdbcVersion,
+ load_flags: LoadFlags,
+ additional_search_paths: Option<Vec<PathBuf>>,
+ profile_provider: impl ConnectionProfileProvider,
+ opts: impl IntoIterator<Item = (<Self as Optionable>::Option,
OptionValue)>,
+ ) -> Result<Self> {
+ let mut drv: ManagedDriver;
+ let result = parse_driver_uri(uri)?;
+ match result {
+ SearchResult::DriverUri(driver, final_uri) => {
+ drv = ManagedDriver::load_from_name(
+ driver,
+ entrypoint,
+ version,
+ load_flags,
+ additional_search_paths.clone(),
+ )?;
Review Comment:
Looks like this is not a trivial change, but you get the intention: move
search logic to `search.rs` and keep only the final instantiation of
`ManagedDriver` in this file.
##########
rust/driver_manager/src/lib.rs:
##########
@@ -100,6 +100,7 @@
// an immutable struct of function pointers. Wrapping the driver in a `Mutex`
// would prevent any parallelism between driver calls, which is not desirable.
+pub mod connection_profiles;
Review Comment:
Use singular `connection_profile`. Or even better, just `profile`.
##########
rust/driver_manager/src/search.rs:
##########
@@ -816,28 +817,221 @@ fn get_search_paths(lvls: LoadFlags) -> Vec<PathBuf> {
result
}
-pub(crate) fn parse_driver_uri(uri: &str) -> Result<(&str, &str)> {
+/// Locates a connection profile file on the filesystem.
+///
+/// This function searches for profile files with a `.toml` extension using the
+/// following strategy:
+///
+/// 1. If `name` is an absolute path with `.toml` extension, verify it exists
+/// 2. If `name` is an absolute path without extension, add `.toml` and return
+/// 3. If `name` is a relative path that exists in the current directory,
return it
+/// 4. Search configured profile directories in order:
+/// - Additional paths provided
+/// - `ADBC_PROFILE_PATH` environment variable paths
+/// - Conda prefix path (if built with `conda_build`)
+/// - User configuration directory
+///
+/// # Arguments
+///
+/// * `name` - Profile name or path (e.g., "my_profile",
"/path/to/profile.toml")
+/// * `additional_path_list` - Optional additional directories to search
+///
+/// # Returns
+///
+/// The absolute path to the located profile file.
+///
+/// # Errors
+///
+/// Returns `Status::NotFound` if the profile cannot be located in any search
path.
+pub(crate) fn find_filesystem_profile(
+ name: impl AsRef<str>,
+ additional_path_list: Option<Vec<PathBuf>>,
+) -> Result<PathBuf> {
+ // Convert the name to a PathBuf to ensure proper platform-specific path
handling.
+ // This normalizes forward slashes to backslashes on Windows.
+ let profile_path = PathBuf::from_slash(name.as_ref());
+ let profile_path = profile_path.as_path();
+
+ // Handle absolute paths
+ if profile_path.is_absolute() {
+ let has_toml_ext = profile_path.extension().is_some_and(|ext| ext ==
"toml");
+
+ if has_toml_ext {
+ // Has .toml extension - verify it exists
+ return if profile_path.is_file() {
+ Ok(profile_path.to_path_buf())
+ } else {
+ Err(Error::with_message_and_status(
+ format!("Profile not found: {}", profile_path.display()),
+ Status::NotFound,
+ ))
Review Comment:
I think you should let the caller try to open the absolute path and provide
an error message (there) about the file failing to open instead of the profile
path not being found. There is nothing to "find" because it's an absolute path.
##########
rust/driver_manager/src/search.rs:
##########
@@ -816,28 +817,221 @@ fn get_search_paths(lvls: LoadFlags) -> Vec<PathBuf> {
result
}
-pub(crate) fn parse_driver_uri(uri: &str) -> Result<(&str, &str)> {
+/// Locates a connection profile file on the filesystem.
+///
+/// This function searches for profile files with a `.toml` extension using the
+/// following strategy:
+///
+/// 1. If `name` is an absolute path with `.toml` extension, verify it exists
+/// 2. If `name` is an absolute path without extension, add `.toml` and return
+/// 3. If `name` is a relative path that exists in the current directory,
return it
+/// 4. Search configured profile directories in order:
+/// - Additional paths provided
+/// - `ADBC_PROFILE_PATH` environment variable paths
+/// - Conda prefix path (if built with `conda_build`)
+/// - User configuration directory
+///
+/// # Arguments
+///
+/// * `name` - Profile name or path (e.g., "my_profile",
"/path/to/profile.toml")
+/// * `additional_path_list` - Optional additional directories to search
+///
+/// # Returns
+///
+/// The absolute path to the located profile file.
+///
+/// # Errors
+///
+/// Returns `Status::NotFound` if the profile cannot be located in any search
path.
+pub(crate) fn find_filesystem_profile(
+ name: impl AsRef<str>,
+ additional_path_list: Option<Vec<PathBuf>>,
+) -> Result<PathBuf> {
+ // Convert the name to a PathBuf to ensure proper platform-specific path
handling.
+ // This normalizes forward slashes to backslashes on Windows.
+ let profile_path = PathBuf::from_slash(name.as_ref());
+ let profile_path = profile_path.as_path();
+
+ // Handle absolute paths
+ if profile_path.is_absolute() {
+ let has_toml_ext = profile_path.extension().is_some_and(|ext| ext ==
"toml");
+
+ if has_toml_ext {
+ // Has .toml extension - verify it exists
+ return if profile_path.is_file() {
+ Ok(profile_path.to_path_buf())
+ } else {
+ Err(Error::with_message_and_status(
+ format!("Profile not found: {}", profile_path.display()),
+ Status::NotFound,
+ ))
+ };
+ }
+
+ // No .toml extension - add it
+ return Ok(profile_path.with_extension("toml"));
+ }
+
+ // For relative paths, check if it's a file at the current location first
+ if profile_path.is_file() {
+ return Ok(profile_path.to_path_buf());
+ }
+
+ // Search in the configured paths
+ let path_list = get_profile_search_paths(additional_path_list);
+ let has_toml_ext = profile_path.extension().is_some_and(|ext| ext ==
"toml");
+
+ path_list
+ .iter()
+ .find_map(|path| {
+ if has_toml_ext {
+ // Name already has .toml extension, use it as-is
+ let full_path = path.join(profile_path);
+ if full_path.is_file() {
+ return Some(full_path);
+ }
+ }
+ // Try adding .toml extension
+ let mut full_path = path.join(profile_path);
+ full_path.set_extension("toml");
+ if full_path.is_file() {
+ return Some(full_path);
+ }
+ None
+ })
+ .ok_or_else(|| {
+ Error::with_message_and_status(
+ format!("Profile not found: {}", name.as_ref()),
+ Status::NotFound,
+ )
+ })
+}
+
+/// Returns the list of directories to search for connection profiles.
+///
+/// Directories are returned in search priority order:
+///
+/// 1. Additional paths provided (highest priority)
+/// 2. `ADBC_PROFILE_PATH` environment variable (colon/semicolon-separated
paths)
+/// 3. Conda prefix path: `$CONDA_PREFIX/etc/adbc/drivers` (if built with
`conda_build`)
+/// 4. User config directory:
+/// - Linux: `~/.config/adbc/profiles`
+/// - macOS: `~/Library/Application Support/ADBC/Profiles`
+/// - Windows: `%LOCALAPPDATA%\ADBC\Profiles`
+///
+/// # Arguments
+///
+/// * `additional_path_list` - Optional additional directories to prepend to
the search path
+///
+/// # Returns
+///
+/// A vector of paths to search for profiles, in priority order.
+fn get_profile_search_paths(additional_path_list: Option<Vec<PathBuf>>) ->
Vec<PathBuf> {
+ let mut result = additional_path_list.unwrap_or_default();
+
+ // Add ADBC_PROFILE_PATH environment variable paths
+ if let Some(paths) = env::var_os("ADBC_PROFILE_PATH") {
+ result.extend(env::split_paths(&paths));
+ }
+
+ // Add conda-specific path if built with conda_build
+ #[cfg(conda_build)]
+ if let Some(conda_prefix) = env::var_os("CONDA_PREFIX") {
+ result.push(
+ PathBuf::from(conda_prefix)
+ .join("etc")
+ .join("adbc")
+ .join("drivers"),
+ );
+ }
+
+ // Add user config directory path
+ #[cfg(any(target_os = "windows", target_os = "macos"))]
+ const PROFILE_DIR_NAME: &str = "Profiles";
+ #[cfg(not(any(target_os = "windows", target_os = "macos")))]
+ const PROFILE_DIR_NAME: &str = "profiles";
+
+ if let Some(profiles_dir) = user_config_dir().and_then(|d|
d.parent().map(|p| p.to_path_buf()))
+ {
+ result.push(profiles_dir.join(PROFILE_DIR_NAME));
+ }
+
+ result
+}
+
+/// Result of parsing a driver URI, indicating how to load the driver.
+///
+/// URIs can specify either a direct driver connection or a profile to load.
+#[derive(Debug)]
+pub(crate) enum SearchResult<'a> {
Review Comment:
There is a struct called `SearchHit` already with the actual loaded library
object. Need a name for this enum that is not so close to `SearchHit` in
meaning. Suggestion `DriverLocator` (as it wraps a universinal resource
LOCATORS).
##########
rust/driver_manager/src/search.rs:
##########
@@ -816,28 +817,221 @@ fn get_search_paths(lvls: LoadFlags) -> Vec<PathBuf> {
result
}
-pub(crate) fn parse_driver_uri(uri: &str) -> Result<(&str, &str)> {
+/// Locates a connection profile file on the filesystem.
+///
+/// This function searches for profile files with a `.toml` extension using the
+/// following strategy:
+///
+/// 1. If `name` is an absolute path with `.toml` extension, verify it exists
+/// 2. If `name` is an absolute path without extension, add `.toml` and return
+/// 3. If `name` is a relative path that exists in the current directory,
return it
+/// 4. Search configured profile directories in order:
+/// - Additional paths provided
+/// - `ADBC_PROFILE_PATH` environment variable paths
+/// - Conda prefix path (if built with `conda_build`)
+/// - User configuration directory
+///
+/// # Arguments
+///
+/// * `name` - Profile name or path (e.g., "my_profile",
"/path/to/profile.toml")
+/// * `additional_path_list` - Optional additional directories to search
+///
+/// # Returns
+///
+/// The absolute path to the located profile file.
+///
+/// # Errors
+///
+/// Returns `Status::NotFound` if the profile cannot be located in any search
path.
+pub(crate) fn find_filesystem_profile(
+ name: impl AsRef<str>,
+ additional_path_list: Option<Vec<PathBuf>>,
+) -> Result<PathBuf> {
+ // Convert the name to a PathBuf to ensure proper platform-specific path
handling.
+ // This normalizes forward slashes to backslashes on Windows.
+ let profile_path = PathBuf::from_slash(name.as_ref());
+ let profile_path = profile_path.as_path();
+
+ // Handle absolute paths
+ if profile_path.is_absolute() {
+ let has_toml_ext = profile_path.extension().is_some_and(|ext| ext ==
"toml");
+
+ if has_toml_ext {
+ // Has .toml extension - verify it exists
+ return if profile_path.is_file() {
+ Ok(profile_path.to_path_buf())
+ } else {
+ Err(Error::with_message_and_status(
+ format!("Profile not found: {}", profile_path.display()),
+ Status::NotFound,
+ ))
+ };
+ }
+
+ // No .toml extension - add it
+ return Ok(profile_path.with_extension("toml"));
Review Comment:
Specially considering that you don't stat the file here. Which is good.
##########
rust/driver_manager/src/search.rs:
##########
@@ -816,28 +817,221 @@ fn get_search_paths(lvls: LoadFlags) -> Vec<PathBuf> {
result
}
-pub(crate) fn parse_driver_uri(uri: &str) -> Result<(&str, &str)> {
+/// Locates a connection profile file on the filesystem.
+///
+/// This function searches for profile files with a `.toml` extension using the
+/// following strategy:
+///
+/// 1. If `name` is an absolute path with `.toml` extension, verify it exists
+/// 2. If `name` is an absolute path without extension, add `.toml` and return
+/// 3. If `name` is a relative path that exists in the current directory,
return it
+/// 4. Search configured profile directories in order:
+/// - Additional paths provided
+/// - `ADBC_PROFILE_PATH` environment variable paths
+/// - Conda prefix path (if built with `conda_build`)
+/// - User configuration directory
+///
+/// # Arguments
+///
+/// * `name` - Profile name or path (e.g., "my_profile",
"/path/to/profile.toml")
+/// * `additional_path_list` - Optional additional directories to search
+///
+/// # Returns
+///
+/// The absolute path to the located profile file.
+///
+/// # Errors
+///
+/// Returns `Status::NotFound` if the profile cannot be located in any search
path.
+pub(crate) fn find_filesystem_profile(
+ name: impl AsRef<str>,
+ additional_path_list: Option<Vec<PathBuf>>,
+) -> Result<PathBuf> {
+ // Convert the name to a PathBuf to ensure proper platform-specific path
handling.
+ // This normalizes forward slashes to backslashes on Windows.
+ let profile_path = PathBuf::from_slash(name.as_ref());
+ let profile_path = profile_path.as_path();
+
+ // Handle absolute paths
+ if profile_path.is_absolute() {
+ let has_toml_ext = profile_path.extension().is_some_and(|ext| ext ==
"toml");
+
+ if has_toml_ext {
+ // Has .toml extension - verify it exists
+ return if profile_path.is_file() {
+ Ok(profile_path.to_path_buf())
+ } else {
+ Err(Error::with_message_and_status(
+ format!("Profile not found: {}", profile_path.display()),
+ Status::NotFound,
+ ))
+ };
+ }
+
+ // No .toml extension - add it
+ return Ok(profile_path.with_extension("toml"));
+ }
+
+ // For relative paths, check if it's a file at the current location first
+ if profile_path.is_file() {
+ return Ok(profile_path.to_path_buf());
+ }
+
+ // Search in the configured paths
+ let path_list = get_profile_search_paths(additional_path_list);
+ let has_toml_ext = profile_path.extension().is_some_and(|ext| ext ==
"toml");
+
+ path_list
+ .iter()
+ .find_map(|path| {
+ if has_toml_ext {
+ // Name already has .toml extension, use it as-is
+ let full_path = path.join(profile_path);
+ if full_path.is_file() {
+ return Some(full_path);
+ }
+ }
+ // Try adding .toml extension
+ let mut full_path = path.join(profile_path);
+ full_path.set_extension("toml");
+ if full_path.is_file() {
+ return Some(full_path);
+ }
+ None
+ })
+ .ok_or_else(|| {
+ Error::with_message_and_status(
+ format!("Profile not found: {}", name.as_ref()),
+ Status::NotFound,
+ )
+ })
+}
+
+/// Returns the list of directories to search for connection profiles.
+///
+/// Directories are returned in search priority order:
+///
+/// 1. Additional paths provided (highest priority)
+/// 2. `ADBC_PROFILE_PATH` environment variable (colon/semicolon-separated
paths)
+/// 3. Conda prefix path: `$CONDA_PREFIX/etc/adbc/drivers` (if built with
`conda_build`)
+/// 4. User config directory:
+/// - Linux: `~/.config/adbc/profiles`
+/// - macOS: `~/Library/Application Support/ADBC/Profiles`
+/// - Windows: `%LOCALAPPDATA%\ADBC\Profiles`
+///
+/// # Arguments
+///
+/// * `additional_path_list` - Optional additional directories to prepend to
the search path
+///
+/// # Returns
+///
+/// A vector of paths to search for profiles, in priority order.
+fn get_profile_search_paths(additional_path_list: Option<Vec<PathBuf>>) ->
Vec<PathBuf> {
+ let mut result = additional_path_list.unwrap_or_default();
+
+ // Add ADBC_PROFILE_PATH environment variable paths
+ if let Some(paths) = env::var_os("ADBC_PROFILE_PATH") {
+ result.extend(env::split_paths(&paths));
+ }
+
+ // Add conda-specific path if built with conda_build
+ #[cfg(conda_build)]
+ if let Some(conda_prefix) = env::var_os("CONDA_PREFIX") {
+ result.push(
+ PathBuf::from(conda_prefix)
+ .join("etc")
+ .join("adbc")
+ .join("drivers"),
+ );
+ }
+
+ // Add user config directory path
+ #[cfg(any(target_os = "windows", target_os = "macos"))]
+ const PROFILE_DIR_NAME: &str = "Profiles";
+ #[cfg(not(any(target_os = "windows", target_os = "macos")))]
+ const PROFILE_DIR_NAME: &str = "profiles";
+
+ if let Some(profiles_dir) = user_config_dir().and_then(|d|
d.parent().map(|p| p.to_path_buf()))
+ {
+ result.push(profiles_dir.join(PROFILE_DIR_NAME));
+ }
+
+ result
+}
+
+/// Result of parsing a driver URI, indicating how to load the driver.
+///
+/// URIs can specify either a direct driver connection or a profile to load.
+#[derive(Debug)]
+pub(crate) enum SearchResult<'a> {
Review Comment:
Then you can drop the `Driver` from the enum entry.
DriverLocator = Uri | ProfilePath
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
