Skip to content

Commit 9933045

Browse files
detulesimonpcouch
andauthored
snowflake: runtime driver config checks (#857)
* snowflake: runtime driver config checks * write less, better code * code-review * code-review: Adding some comments * code-review: Take two - Rather than callback, lean into lazy evaluation ( thanks Simon ) - Warning style updates - [R] style fixes - Extra unit test (single condition warning) * Add news item * Update R/utils.R Co-authored-by: Simon P. Couch <[email protected]> * Update R/driver-databricks.R Co-authored-by: Simon P. Couch <[email protected]> * Update R/utils.R Co-authored-by: Simon P. Couch <[email protected]> * tests: resolve snaps --------- Co-authored-by: Simon P. Couch <[email protected]>
1 parent 190d335 commit 9933045

File tree

8 files changed

+313
-76
lines changed

8 files changed

+313
-76
lines changed

NEWS.md

+3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# odbc (development version)
22

3+
4+
* snowflake: Runtime driver configuration checks on `MacOS` (#857).
5+
36
* Separate column content and name encoding by adding
47
a new `name_encoding` argument to `dbConnect` to complement
58
the existing `encoding` parameter (#845).

R/driver-databricks.R

+29-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ NULL
1717
#' Workbench or the Databricks CLI on desktop.
1818
#' All of these credentials are detected automatically if present using
1919
#' [standard environment variables](https://docs.databricks.com/en/dev-tools/auth.html#environment-variables-and-fields-for-client-unified-authentication).
20+
#' In addition, on macOS platforms, the `dbConnect()` method will check
21+
#' for irregularities with how the driver is configured,
22+
#' and attempt to fix in-situ, unless the `odbc.no_config_override`
23+
#' environment variable is set.
2024
#'
2125
#' @inheritParams DBI::dbConnect
2226
#' @param httpPath,HTTPPath To query a cluster, use the HTTP Path value found
@@ -57,7 +61,6 @@ NULL
5761
#' }
5862
#' @export
5963
databricks <- function() {
60-
configure_spark()
6164
new("DatabricksOdbcDriver")
6265
}
6366

@@ -99,6 +102,9 @@ setMethod("dbConnect", "DatabricksOdbcDriver",
99102
session = session,
100103
...
101104
)
105+
# Perform some sanity checks on MacOS
106+
configure_simba(spark_simba_config(args$driver),
107+
action = "modify")
102108
inject(dbConnect(odbc(), !!!args))
103109
}
104110
)
@@ -343,3 +349,25 @@ workbench_databricks_token <- function(host, cfg_file) {
343349
}
344350
token
345351
}
352+
353+
# p. 44 https://downloads.datastax.com/odbc/2.6.5.1005/Simba%20Spark%20ODBC%20Install%20and%20Configuration%20Guide.pdf
354+
spark_simba_config <- function(driver) {
355+
spark_env <- Sys.getenv("SIMBASPARKINI")
356+
if (!identical(spark_env, "")) {
357+
return(spark_env)
358+
}
359+
common_dirs <- c(
360+
driver_dir(driver),
361+
"/Library/simba/spark/lib",
362+
"/etc",
363+
getwd(),
364+
Sys.getenv("HOME")
365+
)
366+
return(list(
367+
path = list.files(
368+
common_dirs,
369+
pattern = "simba\\.sparkodbc\\.ini$",
370+
full.names = TRUE),
371+
url = "https://www.databricks.com/spark/odbc-drivers-download"
372+
))
373+
}

R/driver-snowflake.R

+36-2
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,10 @@ setMethod("odbcDataType", "Snowflake",
9898
#'
9999
#' In particular, the custom `dbConnect()` method for the Snowflake ODBC driver
100100
#' detects ambient OAuth credentials on platforms like Snowpark Container
101-
#' Services or Posit Workbench.
101+
#' Services or Posit Workbench. In addition, on macOS platforms, the
102+
#' `dbConnect` method will check and warn if it detects irregularities with
103+
#' how the driver is configured, unless the `odbc.no_config_override`
104+
#' environment variable is set.
102105
#'
103106
#' @inheritParams DBI::dbConnect
104107
#' @param account A Snowflake [account
@@ -185,6 +188,9 @@ setMethod(
185188
session = session,
186189
...
187190
)
191+
# Perform some sanity checks on MacOS
192+
configure_simba(snowflake_simba_config(args$driver),
193+
action = "modify")
188194
inject(dbConnect(odbc(), !!!args))
189195
}
190196
)
@@ -260,13 +266,41 @@ snowflake_default_driver_paths <- function() {
260266
"/usr/lib/snowflake/odbc/lib/libSnowflake.so"
261267
)
262268
} else if (Sys.info()["sysname"] == "Darwin") {
263-
paths <- "/opt/snowflake/snowflakeodbc/lib/universal/libSnowflake.dylib"
269+
paths <- c(
270+
"/opt/snowflake/snowflakeodbc/lib/universal/libSnowflake.dylib",
271+
"/opt/snowflake-osx-x64/bin/lib/libsnowflakeodbc_sb64-universal.dylib"
272+
)
264273
} else {
265274
paths <- character()
266275
}
267276
paths[file.exists(paths)]
268277
}
269278

279+
snowflake_simba_config <- function(driver) {
280+
snowflake_env <- Sys.getenv("SIMBASNOWFLAKEINI")
281+
if (!identical(snowflake_env, "")) {
282+
return(snowflake_env)
283+
}
284+
# Posit configuration is likely at:
285+
# /opt/snowflake-osx-x64/bin/lib/rstudio.snowflakeodbc.ini
286+
# OEM configuration is likely at:
287+
# /opt/snowflake/snowflakeodbc/lib/universal/simba.snowflake.ini
288+
common_dirs <- c(
289+
driver_dir(driver),
290+
"/opt/snowflake-osx-x64/bin/lib/",
291+
"/opt/snowflake/snowflakeodbc/lib/universal/",
292+
getwd(),
293+
Sys.getenv("HOME")
294+
)
295+
return(list(
296+
path = list.files(
297+
simba_config_dirs(driver),
298+
pattern = "snowflake(odbc)?\\.ini$",
299+
full.names = TRUE),
300+
url = "https://docs.snowflake.com/en/developer-guide/odbc/odbc-download"
301+
))
302+
}
303+
270304
snowflake_server <- function(account) {
271305
if (nchar(account) == 0) {
272306
abort(

R/utils.R

+95-50
Original file line numberDiff line numberDiff line change
@@ -288,28 +288,44 @@ check_attributes <- function(attributes, call = caller_env()) {
288288
}
289289

290290
# apple + spark drive config (#651) --------------------------------------------
291-
configure_spark <- function(call = caller_env()) {
291+
# Method will attempt to:
292+
# 1. Locate an installation of unixodbc / error out otherwise.
293+
# 2. Verify the driver_config argument. Expect this to be a list with
294+
# two fields:
295+
# * path Vector of viable driver paths ( only first one is used )
296+
# * url A location where the user can downlaod the driver from.
297+
# See spark_simba_config, for example. Its return value is used as
298+
# the value for this argument.
299+
# 3. Inspect the config for some settings that can impact how our package
300+
# performs.
301+
# 4. If action == "modify" then we attempt to modify the config in-situ.
302+
# 5. Otherwise we throw a warning asking the user to revise.
303+
configure_simba <- function(driver_config,
304+
action = "modify", call = caller_env()) {
292305
if (!is_macos()) {
293306
return(invisible())
294307
}
308+
if (!is.null(getOption("odbc.no_config_override"))) {
309+
return(invisible())
310+
}
295311

296312
unixodbc_install <- locate_install_unixodbc()
297313
if (length(unixodbc_install) == 0) {
298314
error_install_unixodbc(call)
299315
}
300316

301-
spark_config <- locate_config_spark()
302-
if (length(spark_config) == 0) {
303-
abort(
304-
c(
305-
"Unable to locate the needed spark ODBC driver.",
306-
i = "Please install the needed driver from https://www.databricks.com/spark/odbc-drivers-download."
307-
),
317+
simba_config <- driver_config$path
318+
if (length(simba_config) == 0) {
319+
func <- cli::warn
320+
if (action == "modify") {
321+
fun <- cli::abort
322+
}
323+
func(
324+
c(i = "Please install the needed driver from {driver_config$url}"),
308325
call = call
309326
)
310327
}
311-
312-
configure_unixodbc_spark(unixodbc_install[1], spark_config[1], call)
328+
configure_unixodbc_simba(unixodbc_install[1], simba_config[1], action, call)
313329
}
314330

315331
locate_install_unixodbc <- function() {
@@ -369,51 +385,51 @@ error_install_unixodbc <- function(call) {
369385
)
370386
}
371387

372-
# p. 44 https://downloads.datastax.com/odbc/2.6.5.1005/Simba%20Spark%20ODBC%20Install%20and%20Configuration%20Guide.pdf
373-
locate_config_spark <- function() {
374-
spark_env <- Sys.getenv("SIMBASPARKINI")
375-
if (!identical(spark_env, "")) {
376-
return(spark_env)
377-
}
378-
379-
common_dirs <- c(
380-
"/Library/simba/spark/lib",
381-
"/etc",
382-
getwd(),
383-
Sys.getenv("HOME")
384-
)
388+
configure_unixodbc_simba <- function(unixodbc_install, simba_config, action, call) {
385389

386-
list.files(
387-
common_dirs,
388-
pattern = "simba\\.sparkodbc\\.ini$",
389-
full.names = TRUE
390-
)
391-
}
392-
393-
configure_unixodbc_spark <- function(unixodbc_install, spark_config, call) {
394390
# As shipped, the simba spark ini has an incomplete final line
395391
suppressWarnings(
396-
spark_lines <- readLines(spark_config)
392+
simba_lines <- readLines(simba_config)
397393
)
398-
399-
spark_lines_new <- replace_or_append(
400-
lines = spark_lines,
401-
pattern = "^ODBCInstLib=",
394+
res <- replace_or_append(
395+
lines = simba_lines,
396+
key_pattern = "^ODBCInstLib=",
397+
accepted_value = unixodbc_install,
402398
replacement = paste0("ODBCInstLib=", unixodbc_install)
403399
)
404-
405-
spark_lines_new <- replace_or_append(
406-
lines = spark_lines_new,
407-
pattern = "^DriverManagerEncoding=",
400+
warnings <- character()
401+
if (action != "modify" && res$modified) {
402+
warnings <- c(warnings, c("*" = "Please consider revising the {.arg ODBCInstLib}
403+
field in {.file {simba_config}} and setting its value to {.val {unixodbc_install}}"))
404+
}
405+
simba_lines_new <- res$new_lines
406+
res <- replace_or_append(
407+
lines = simba_lines_new,
408+
key_pattern = "^DriverManagerEncoding=",
409+
accepted_value = "UTF-16|utf-16",
408410
replacement = "DriverManagerEncoding=UTF-16"
409411
)
412+
if (action != "modify" && res$modified) {
413+
warnings <- c(warnings, c("*" = "Please consider revising the
414+
{.arg DriverManagerEncoding} field in {.file {simba_config}} and setting its
415+
value to {.val UTF-16}."))
416+
}
417+
if (length(warnings)) {
418+
cli::cli_warn(c(
419+
c(i = "Detected potentially unsafe driver settings:"),
420+
warnings
421+
))
422+
}
423+
simba_lines_new <- res$new_lines
410424

411-
write_spark_lines(spark_lines, spark_lines_new, spark_config, call)
425+
if (action == "modify") {
426+
write_simba_lines(simba_lines, simba_lines_new, simba_config, call)
427+
}
412428

413429
invisible()
414430
}
415431

416-
write_spark_lines <- function(spark_lines, spark_lines_new, spark_config, call) {
432+
write_simba_lines <- function(spark_lines, spark_lines_new, spark_config, call) {
417433
if (identical(spark_lines, spark_lines_new)) {
418434
return(invisible())
419435
}
@@ -436,23 +452,52 @@ write_spark_lines <- function(spark_lines, spark_lines_new, spark_config, call)
436452
writeLines(spark_lines_new, spark_config)
437453
}
438454

455+
# Interpret the argument as an `ODBC` driver
456+
# and attempt to infer the directory housing it.
457+
# It will return an empty character vector if unable to.
458+
driver_dir <- function(driver) {
459+
# driver argument could be an outright path, or a name
460+
# of a driver specified in odbcinst.ini Try to discern
461+
driver_spec <- subset(odbcListDrivers(), name == driver)
462+
if (nrow(driver_spec)) {
463+
driver_path <- subset(driver_spec, attribute == "Driver")$value
464+
} else {
465+
driver_path <- driver
466+
}
467+
468+
ret <- dirname(driver_path)
469+
if (ret == ".") {
470+
ret <- character()
471+
}
472+
return(ret)
473+
}
474+
439475
is_writeable <- function(path) {
440476
tryCatch(file.access(path, mode = 2)[[1]] == 0, error = function(e) FALSE)
441477
}
442478

443-
# given a vector of lines in an ini file, look for a given key pattern.
444-
# the `replacement` is the whole intended line, giving the "key=value" pair.
445-
# if the key is found, replace that line with `replacement`.
446-
# if the key isn't found, append a new line with `replacement`.
447-
replace_or_append <- function(lines, pattern, replacement) {
448-
matching_lines_loc <- grepl(pattern, lines)
479+
# Given a vector of lines in an ini file, look for a given key pattern.
480+
# If found:
481+
# - No action if the `accepted_value` argument is found on line.
482+
# - Replace otherwise.
483+
# If not found: append.
484+
# The `replacement` is the whole intended line, giving the desired
485+
# "key=value" pair.
486+
# @return a list with two elements:
487+
# - new_lines: Potentially modified lines
488+
# - modified: Whether method modified lines, where modified means
489+
# both changed or appended.
490+
replace_or_append <- function(lines, key_pattern, accepted_value, replacement) {
491+
matching_lines_loc <- grepl(key_pattern, lines)
449492
matching_lines <- lines[matching_lines_loc]
493+
found_ok <- length(matching_lines) != 0 &&
494+
any(grepl(accepted_value, lines[matching_lines_loc]))
450495
if (length(matching_lines) == 0) {
451496
lines <- c(lines, replacement)
452-
} else {
497+
} else if (!found_ok) {
453498
lines[matching_lines_loc] <- replacement
454499
}
455-
lines
500+
return(list(new_lines = lines, modified = !found_ok))
456501
}
457502

458503
check_shiny_session <- function(x,

man/databricks.Rd

+4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

man/snowflake.Rd

+4-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)