Skip to content

Commit

Permalink
chore(r/adbcsnowflake): Update adbcsnowflake such that it can be subm…
Browse files Browse the repository at this point in the history
…itted to CRAN (#1220)

Closes #1067.

The mailing list thread where I asked about this (
https://stat.ethz.ch/pipermail/r-package-devel/2023q3/009329.html )
noted a few possible requirements:

- Make sure to not download dependencies. If you do need to download,
bundle the dependencies separately and download them at install time
(with checksum verification). Because our dependencies are >5MB zipped
and contain paths longer than 100 characters, we need to do this. I
wired this up so you can test it locally by providing a `file://`
URL...when submission times come that URL and shasum can be hardwired
into the package.
- Make sure to not download Go itself and make sure to check Go
locations that might not be on PATH. I removed the go download for
now...if it becomes too hard of a constraint to satisfy it could be
reinstated and removed just prior to submission. Luckily, installing Go
is pretty straightforward.

I also added some updates to make the tests run with my local Snowflake
account and consoliated the Windows and MacOS and Linux builds since the
all use the same logic.

This will probably need to be workshopped during submission and, after
successful submission, distilled into some common place and applied to
the adbcflightsql package.
  • Loading branch information
paleolimbot authored Oct 30, 2023
1 parent 3a8f6dc commit fb5e53c
Show file tree
Hide file tree
Showing 15 changed files with 271 additions and 70 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/r-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ jobs:

steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v4
with:
go-version: '1.20'
- uses: r-lib/actions/setup-r@v2
with:
r-version: release
Expand Down
1 change: 0 additions & 1 deletion r/adbcsnowflake/.Rbuildignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
^bootstrap\.R$
^README\.Rmd$
^src/Makevars$
^configure\.win$
^\.vscode$
^src/go/tmp$
^_pkgdown\.yml$
Expand Down
2 changes: 1 addition & 1 deletion r/adbcsnowflake/DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.2.3
Suggests:
arrow,
nanoarrow,
testthat (>= 3.0.0)
SystemRequirements: GNU make
Config/testthat/edition: 3
Config/build/bootstrap: TRUE
URL: https://github.com/apache/arrow-adbc
Expand Down
40 changes: 40 additions & 0 deletions r/adbcsnowflake/R/adbcsnowflake-package.R
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,43 @@ adbc_statement_init.adbcsnowflake_connection <- function(connection, ...,
subclass = "adbcsnowflake_statement"
)
}

adbcsnowflake_test_db <- function(...) {
args <- c(list(adbcsnowflake()), test_db_options(...))
do.call(adbc_database_init, args)
}

adbcsnowflake_has_test_db <- function() {
tryCatch({test_db_options(); TRUE}, error = function(e) FALSE)
}

test_db_options <- function(...) {
uri <- Sys.getenv("ADBC_SNOWFLAKE_TEST_URI", "")
if (!identical(uri, "")) {
return(list(uri = uri, ...))
}

account <- Sys.getenv("ADBC_SNOWFLAKE_TEST_ACCOUNT", "")
username <- Sys.getenv("ADBC_SNOWFLAKE_TEST_USERNAME", "")
auth_type <- Sys.getenv("ADBC_SNOWFLAKE_TEST_AUTH_TYPE", "auth_ext_browser")

if (identical(account, "") || identical(username, "")) {
stop(
paste(
"adbcsnowflake tests can be run by setting ADBC_SNOWFLAKE_TEST_URI",
"or by setting ADBC_SNOWFLAKE_TEST_ACCOUNT and ADBC_SNOWFLAKE_TEST_USERNAME,",
"which supports single-sign on (SSO) authentication via the browser.",
"You can set these environment variables in ~/.Renviron.",
"See https://arrow.apache.org/adbc/current/driver/snowflake.html",
"for more information."
)
)
}

list(
adbc.snowflake.sql.account = account,
username = username,
adbc.snowflake.sql.auth_type = auth_type,
...
)
}
4 changes: 2 additions & 2 deletions r/adbcsnowflake/README.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ pak::cache_clean()
## Example

This is a basic example which shows you how to solve a common problem. For examples
of `uri` values to use as a connection value, see the documentation for the
[upstream Go driver implementation](https://github.com/apache/arrow-adbc/blob/main/docs/source/driver/go/snowflake.rst#uri-format).
of `uri` values to use as a connection value, see the
[Snowflake driver documentation](https://arrow.apache.org/adbc/current/driver/snowflake.html#uri-format). Single sign-on browser-based authentication is also supported.

```{r example}
library(adbcdrivermanager)
Expand Down
7 changes: 3 additions & 4 deletions r/adbcsnowflake/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,9 @@ pak::cache_clean()

## Example

This is a basic example which shows you how to solve a common problem.
For examples of `uri` values to use as a connection value, see the
documentation for the [upstream Go driver
implementation](https://github.com/apache/arrow-adbc/blob/main/docs/source/driver/go/snowflake.rst#uri-format).
This is a basic example which shows you how to solve a common problem. For examples
of `uri` values to use as a connection value, see the
[Snowflake driver documentation](https://arrow.apache.org/adbc/current/driver/snowflake.html#uri-format). Single sign-on browser-based authentication is also supported.

``` r
library(adbcdrivermanager)
Expand Down
101 changes: 83 additions & 18 deletions r/adbcsnowflake/configure
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@
R_BIN="$R_HOME/bin/R"
RSCRIPT_BIN="$R_HOME/bin/Rscript"

# Set to non-false for CRAN releases, which require this approach to comply with
# guidance regarding large dependency sources. When non-false, this script
# will fail if the download fails or if checksum verification fails on
# the downloaded file.
DOWNLOAD_DEPENDENCY_ARCHIVE="false"

# Run bootstrap.R. This will have already run if we are installing a source
# package built with pkgbuild::build() with pkgbuild >1.4.0; however, we
# run it again in case this is R CMD INSTALL on a directory or
Expand All @@ -28,27 +34,79 @@ if [ -f bootstrap.R ]; then
"$RSCRIPT_BIN" bootstrap.R
fi

# If we need to fetch an archive of the vendored dependencies, do so now
if [ ! "$DOWNLOAD_DEPENDENCY_ARCHIVE" = "false" ]; then
"$RSCRIPT_BIN" tools/download-go-vendor-archive.R
if [ ! -f tools/src-go-adbc-vendor.zip ]; then
echo "Failed to fetch vendored dependency archive"
exit 1
fi
fi

# If we've fetched an archive of vendored dependencies (or built one for testing)
# extract it into src/go/adbc/vendor to use the archived/vendored dependencies.
# You can run Rscript tools/create-go-vendor-archive.zip to generate the
# dependency archive and checksum file and place it in the tools/ directory.
if [ -f tools/src-go-adbc-vendor.zip ]; then
if [ -z "$SHASUM_BIN" ]; then
SHASUM_BIN=`which shasum`
fi

DEFAULT_SHASUM_RTOOLS="/usr/bin/core_perl/shasum"
if [ -z "$SHASUM_BIN" ] && [ -f "$DEFAULT_SHASUM_RTOOLS" ]; then
SHASUM_BIN="$DEFAULT_SHASUM_RTOOLS"
fi

cd tools
if "$SHASUM_BIN" --algorithm 512 --check --status src-go-adbc-vendor.zip.sha512 ; then
cd ..
"$RSCRIPT_BIN" tools/extract-go-vendor-archive.R
else
echo "Checksum verification failed for vendored dependency archive"
exit 1
fi
fi

# Find the go binary so that we can go build!
# If we've downloaded a specific version of go to src/go/tmp, use that
# one (helpful for testing different version of go locally)
PREVIOUSLY_DOWNLOADED_GO="`pwd`/src/go/tmp/go/bin/go"
if [ -z "$GO_BIN" ] && [ -f "$PREVIOUSLY_DOWNLOADED_GO" ]; then
GO_BIN="$PREVIOUSLY_DOWNLOADED_GO"
fi

# Check go on PATH
if [ -z "$GO_BIN" ]; then
GO_BIN=`which go`
fi

if [ -z "$GO_BIN" ]; then
if [ ! -f src/go/tmp/go/bin/go ]; then
echo ""
echo "Downloading and extracting Go into the package source directory:"
echo "This may take a few minutes. To eliminate this step, install Go"
echo "from your faviourite package manager or set the GO_BIN environment variable:"
echo "- apt-get install golang"
echo "- brew install golang"
echo "- dnf install golang"
echo "- apk add go"
echo "- pacman -S go"

"$RSCRIPT_BIN" tools/download-go.R
fi
# Try some default install locations that might not be on PATH
DEFAULT_GO_WIN="C:/Program Files/Go/bin/go.exe"
if [ -z "$GO_BIN" ] && [ -f "$DEFAULT_GO_WIN" ]; then
GO_BIN="$DEFAULT_GO_WIN"
fi

GO_BIN="`pwd`/src/go/tmp/go/bin/go"
DEFAULT_GO_MACOS="/usr/local/go/bin/go"
if [ -z "$GO_BIN" ] && [ -f "$DEFAULT_GO_MACOS" ]; then
GO_BIN="$DEFAULT_GO_MACOS"
fi

DEFAULT_GO_HOMEBREW_M1="/opt/homebrew/bin/go"
if [ -z "$GO_BIN" ] && [ -f "$DEFAULT_GO_HOMEBREW_M1" ]; then
GO_BIN="$DEFAULT_GO_HOMEBREW_M1"
fi

if [ -z "$GO_BIN" ]; then
echo ""
echo "The Go compiler is required to install this package. You can install go"
echo "from your faviourite package manager or set the GO_BIN environment variable:"
echo "- apt-get install golang"
echo "- brew install golang"
echo "- dnf install golang"
echo "- apk add go"
echo "- pacman -S go"
echo "...or from the official installers available at https://go.dev/dl/"
exit 1
fi

echo "Trying 'go version' with GO_BIN at '$GO_BIN'"
Expand All @@ -65,22 +123,29 @@ CXX=`"$R_BIN" CMD config CXX`
# clang and gcc use different symbol-hiding syntax and we need to
# make sure to hide any Adbc* symbols that might conflict with another
# driver.
if "$R_BIN" CMD config CC | grep -e "clang" ; then
if "$R_BIN" CMD config CC | grep -e "clang" >/dev/null ; then
SYMBOL_ARGS="-Wl,-exported_symbol,_adbcsnowflake_c_snowflake -Wl,-exported_symbol,_R_init_adbcsnowflake"
elif "$R_BIN" CMD config CC | grep -e "gcc" ; then
elif "$R_BIN" CMD config CC | grep -e "gcc" >/dev/null ; then
SYMBOL_ARGS="-Wl,--version-script=go/symbols.map"
fi

# On OSX we need -framework Security because of some dependency somewhere
if [ `uname` = "Darwin" ]; then
PKG_LIBS="-framework Security $PKG_LIBS"
PKG_LIBS="-framework Security -lresolv $PKG_LIBS"
elif uname | grep -e "MSYS" >/dev/null ; then
# Windows
PKG_LIBS="$PKG_LIBS"
else
# Linux
PKG_LIBS="-lresolv $PKG_LIBS"
fi

PKG_LIBS="$PKG_LIBS $SYMBOL_ARGS"

sed \
-e "s|@gobin@|$GO_BIN|" \
-e "s|@libs@|$PKG_LIBS|" \
-e "s|@cflags@|$PKG_CPPFLAGS|" \
-e "s|@cc@|$CC|" \
-e "s|@cxx@|$CXX|" \
src/Makevars.in > src/Makevars
Expand Down
9 changes: 3 additions & 6 deletions r/adbcsnowflake/configure.win
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
# specific language governing permissions and limitations
# under the License.

# See configure for a description of this process.
# This is only for development: this file and bootstrap.R will be removed
# prior to packaging
if [ -f bootstrap.R ]; then
$R_HOME/bin/Rscript bootstrap.R
fi
# On Windows, this package only compiles on R >= 4.2 where the normal
# configure is sufficient
./configure
4 changes: 2 additions & 2 deletions r/adbcsnowflake/src/Makevars.in
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
# specific language governing permissions and limitations
# under the License.

PKG_CPPFLAGS=-I$(CURDIR)/src -DADBC_EXPORT=""
PKG_LIBS=-L$(CURDIR)/go -ladbc_driver_snowflake -lresolv @libs@
PKG_CPPFLAGS=-DADBC_EXPORT="" @cflags@
PKG_LIBS=-L$(CURDIR)/go -ladbc_driver_snowflake @libs@

CGO_CC = @cc@
CGO_CXX = @cxx@
Expand Down
24 changes: 4 additions & 20 deletions r/adbcsnowflake/tests/testthat/test-adbcsnowflake-package.R
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,9 @@ test_that("adbcsnowflake() works", {
})

test_that("default options can open a database and execute a query", {
test_db_uri <- Sys.getenv("ADBC_SNOWFLAKE_TEST_URI", "")
skip_if(identical(test_db_uri, ""))
skip_if_not(adbcsnowflake_has_test_db())

db <- adbcdrivermanager::adbc_database_init(
adbcsnowflake(),
uri = test_db_uri
)
db <- adbcsnowflake_test_db()
expect_s3_class(db, "adbcsnowflake_database")

con <- adbcdrivermanager::adbc_connection_init(db)
Expand All @@ -37,30 +33,18 @@ test_that("default options can open a database and execute a query", {
adbcdrivermanager::adbc_database_release(db)
})

stmt <- adbcdrivermanager::adbc_statement_init(con)
expect_s3_class(stmt, "adbcsnowflake_statement")
adbcdrivermanager::adbc_statement_set_sql_query(
stmt,
"use schema snowflake_sample_data.tpch_sf1;"
)
adbcdrivermanager::adbc_statement_execute_query(stmt)
adbcdrivermanager::adbc_statement_release(stmt)

stmt <- adbcdrivermanager::adbc_statement_init(con)
expect_s3_class(stmt, "adbcsnowflake_statement")

adbcdrivermanager::adbc_statement_set_sql_query(
stmt,
"SELECT R_REGIONKEY FROM REGION ORDER BY R_REGIONKEY"
"SELECT 'abc' as ABC"
)

stream <- nanoarrow::nanoarrow_allocate_array_stream()
adbcdrivermanager::adbc_statement_execute_query(stmt, stream)
result <- as.data.frame(stream)
expect_identical(
result$R_REGIONKEY,
c(0, 1, 2, 3, 4)
)
expect_identical(result, data.frame(ABC = "abc"))

adbcdrivermanager::adbc_statement_release(stmt)
})
18 changes: 18 additions & 0 deletions r/adbcsnowflake/tools/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

src-go-adbc-vendor.zip*
Loading

0 comments on commit fb5e53c

Please sign in to comment.