Skip to content

Commit 4f1c0b6

Browse files
Rewrite (#41)
This PR does a very large refactor (aka rewrite) of the driver do prepare for general availability. ## Design Considerations * Any request to external systems must return as soon as the context deadline is exceeded * Any request to external systems that do not take a context (open/close session) must have a timeout * Packages that are not meant to be public to users must be in /internal * No extension of the standard interface as context cancellation fulfills the requirements; thus, concepts such as “operation” or “cursor” are not needed. * Contract testing instead of full integration testing (DUST). This enables all tests to run locally and in Github actions. And they are fast. This is done by creating a test thrift server using httptest package. * Implement RoundTripper interface and wrap TCLIServiceClient to have access to HTTP response headers and status. This will enable retry. * Fully declarative configuration with defaults instead of constants spreaded around ## New features * Support for operation cancellation and timeout by using context. * Direct Results. If the resultsset is small and the query takes less than 5 seconds, there will only be one request made, shortcutting 3 or 4 requests from the standard thrift protocol. * Support for session parameters (maxRows, timeout, catalog, schema, and any other session params in SET such as timezone, ansi_mode, etc). * Fully configurable logging ## Regressions * Parameterized queries are not supported anymore. Client side string interpolation is prone to errors and Databricks will soon support it from the server side anyway. * DSN does not start with "databricks" anymore. So one can just do `username:password@host:port/path/to/db?param=foo` ## Testing There are 3 levels of testing: * Unit: see internal/config/config_test.go * Contract: see rows_test.go * End to End with test server: see driver_e2e_test.go ## Bug fixes * Many with regards to how to query, as synchronous queries were never really supported in the server. * Some data type supported conversions where fixed ## Examples ```go package main import ( "context" "database/sql" "fmt" "log" "os" "time" _ "github.com/databricks/databricks-sql-go" ) func main() { // Opening a driver typically will not attempt to connect to the database. db, err := sql.Open("databricks", <DSN>) if err != nil { // This will not be a connection error, but a DSN parse error or // another initialization error. log.Fatal(err) } defer db.Close() ctx, cancel1 := context.WithTimeout(context.Background(), 10*time.Second) defer cancel1() var res int err := db.QueryRowContext(ctx, `SELECT id FROM RANGE(100000000) ORDER BY RANDOM() + 2 asc`).Scan(&res) if err != nil { fmt.Printf("err: %v\n", err1) } else { fmt.Printf("result: %d\n", res) } } ``` ```go package main import ( "context" "database/sql" "fmt" "log" "os" "strconv" dbsql "github.com/databricks/databricks-sql-go" ) func main() { port, err := strconv.Atoi(os.Getenv("DATABRICKS_PORT")) if err != nil { log.Fatal(err.Error()) } connector, err := dbsql.NewConnector( dbsql.WithServerHostname(os.Getenv("DATABRICKS_HOST")), dbsql.WithPort(port), dbsql.WithHTTPPath(os.Getenv("DATABRICKS_HTTPPATH")), dbsql.WithAccessToken(os.Getenv("DATABRICKS_ACCESSTOKEN")), dbsql.WithSessionParams(map[string]string{"timezone": "America/Sao_Paulo", "ansi_mode": "true"}), ) if err != nil { // This will not be a connection error, but a DSN parse error or // another initialization error. log.Fatal(err) } db := sql.OpenDB(connector) defer db.Close() ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() res := &time.Time{} if qerr := db.QueryRowContext(ctx, `select now()`).Scan(res); qerr != nil { fmt.Printf("err: %v\n", qerr) } else { fmt.Println(res) } } ``` ## TODO * Testing is still in a minimum, even though we added a lot more tests than previously * Retry. The interfaces are all there, we just need to write the logic to retry now. Signed-off-by: Andre Furlan <[email protected]> Co-authored-by: Raymond Cypher <[email protected]>
1 parent ab55104 commit 4f1c0b6

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+3886
-1717
lines changed

.github/CODEOWNERS

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@
22
# the repo. Unless a later match takes precedence, these
33
# users will be requested for review when someone opens a
44
# pull request.
5-
* @arikfr @susodapop @moderakh @yunbodeng-db
5+
* @mattdeekay @susodapop @moderakh @yunbodeng-db @andrefurlan-db @rcypher-databricks
6+

.gitignore

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
# Test binary, built with `go test -c`
1111
*.test
12+
test-results/
1213

1314
# Output of the go coverage tool, specifically when used with LiteIDE
1415
*.out
@@ -45,3 +46,21 @@ Icon
4546
Network Trash Folder
4647
Temporary Items
4748
.apdisk
49+
50+
51+
.env
52+
53+
# Output of the production build
54+
bin/
55+
56+
57+
# Dependency directories
58+
vendor/
59+
60+
*.log
61+
*.bak
62+
_tmp*
63+
64+
.vscode/
65+
__debug_bin
66+
.DS_Store

.golangci.yml

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
linters:
2+
disable-all: true
3+
enable:
4+
- bodyclose
5+
- deadcode
6+
- depguard
7+
- dogsled
8+
# - dupl
9+
- errcheck
10+
# - exportloopref
11+
# - funlen
12+
# - gochecknoinits
13+
# - goconst
14+
# - gocritic
15+
# - gocyclo
16+
- gofmt
17+
# - goimports
18+
# - gomnd
19+
- goprintffuncname
20+
- gosec
21+
- gosimple
22+
- govet
23+
- ineffassign
24+
# - lll
25+
# - misspell
26+
- nakedret
27+
# - noctx
28+
- nolintlint
29+
- staticcheck
30+
- structcheck
31+
# - stylecheck
32+
- typecheck
33+
# - unconvert
34+
# - unparam
35+
- unused
36+
- varcheck
37+
# - whitespace
38+
39+
# don't enable:
40+
# - asciicheck
41+
# - scopelint
42+
# - gochecknoglobals
43+
# - gocognit
44+
# - godot
45+
# - godox
46+
# - goerr113
47+
# - interfacer
48+
# - maligned
49+
# - nestif
50+
# - prealloc
51+
# - testpackage
52+
# - revive
53+
# - wsl
54+
55+
linters-settings:
56+
gosec:
57+
exclude-generated: true
58+
severity: "low"
59+
confidence: "low"
60+
61+
run:
62+
timeout: 5m

Makefile

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
BINARY = vulcan
2+
PACKAGE = github.com/datajoydev/$(BINARY)
3+
VER_PREFIX = $(PACKAGE)/version
4+
PRODUCTION ?= false
5+
DATE = $(shell date "+%Y-%m-%d")
6+
GIT_BRANCH = $(shell git rev-parse --abbrev-ref 2>/dev/null)
7+
GIT_COMMIT = $(shell git rev-parse HEAD 2>/dev/null)
8+
GIT_TAG = $(shell if [ -z "`git status --porcelain`" ]; then git describe --exact-match --tags HEAD 2>/dev/null; fi)
9+
VERSIONREL = $(shell if [ -z "`git status --porcelain`" ]; then git rev-parse --short HEAD 2>/dev/null ; else echo "dirty"; fi)
10+
PKGS = $(shell go list ./... | grep -v /vendor)
11+
LDFLAGS = -X $(VER_PREFIX).Version=$(VERSIONREL) -X $(VER_PREFIX).Revision=$(GIT_COMMIT) -X $(VER_PREFIX).Branch=$(GIT_BRANCH) -X $(VER_PREFIX).BuildUser=$(shell whoami) -X $(VER_PREFIX).BuildDate=$(shell date -u +"%Y-%m-%dT%H:%M:%SZ")
12+
GOBUILD_ARGS =
13+
GO = CGO_ENABLED=0 go
14+
PLATFORMS = windows linux darwin
15+
os = $(word 1, $@)
16+
17+
TEST_RESULTS_DIR ?= ./test-results
18+
19+
20+
ifneq (${GIT_TAG},)
21+
override DOCKER_TAG = ${GIT_TAG}
22+
override VERSIONREL = ${GIT_TAG}
23+
endif
24+
25+
ifeq (${PRODUCTION}, true)
26+
LDFLAGS += -w -s -extldflags "-static"
27+
GOBUILD_ARGS += -v
28+
endif
29+
30+
.PHONY: help
31+
help: ## Show this help.
32+
@awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z_-]+:.*?## / {sub("\\\\n",sprintf("\n%22c"," "), $$2);printf "\033[36m%-20s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST)
33+
34+
.PHONY: all
35+
all: gen fmt lint test coverage ## format and test everything
36+
37+
bin/golangci-lint: go.mod go.sum
38+
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b ./bin v1.48.0
39+
40+
bin/gotestsum: go.mod go.sum
41+
@mkdir -p bin/
42+
$(GO) build -o bin/gotestsum gotest.tools/gotestsum
43+
44+
.PHONY: tools
45+
tools: bin/golangci-lint bin/gotestsum ## Build the development tools
46+
47+
.PHONY: fmt
48+
fmt: ## Format the go code.
49+
gofmt -w -s .
50+
51+
.PHONY: lint
52+
lint: bin/golangci-lint ## Lint the go code to ensure code sanity.
53+
./bin/golangci-lint run
54+
55+
.PHONY: test
56+
test: bin/gotestsum ## Run the go unit tests.
57+
@echo "INFO: Running all go unit tests."
58+
CGO_ENABLED=0 ./bin/gotestsum --format pkgname-and-test-fails --junitfile $(TEST_RESULTS_DIR)/unit-tests.xml ./...
59+
60+
.PHONY: coverage
61+
coverage: bin/gotestsum ## Report the unit test code coverage.
62+
@echo "INFO: Generating unit test coverage report."
63+
CGO_ENABLED=0 ./bin/gotestsum --format pkgname-and-test-fails -- -coverprofile=$(TEST_RESULTS_DIR)/coverage.txt -covermode=atomic ./...
64+
go tool cover -html=$(TEST_RESULTS_DIR)/coverage.txt -o $(TEST_RESULTS_DIR)/coverage.html
65+
66+
.PHONY: sec
67+
sec: lint ## Run the snyk vulnerability scans.
68+
@echo "INFO: Running go vulnerability scans."
69+
snyk test .
70+
71+
.PHONY: build
72+
build: linux darwin ## Build the multi-arch binaries
73+
74+
.PHONY: $(PLATFORMS)
75+
$(PLATFORMS):
76+
mkdir -p bin
77+
GOOS=$(os) GOARCH=amd64 $(GO) build $(GOBUILD_ARGS) -ldflags '$(LDFLAGS)' -o bin/$(BINARY)-$(os)-amd64 .

NOTICE

Lines changed: 0 additions & 35 deletions
This file was deleted.

README.md

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,22 +48,24 @@ You can set HTTP Timeout value by appending a `timeout` query parameter (in mill
4848
databricks://:[your token]@[Workspace hostname][Endpoint HTTP Path]?timeout=1000&maxRows=1000
4949
```
5050

51-
## Testing
52-
51+
## Develop
52+
53+
### Lint
54+
We use `golangci-lint` as the lint tool. If you use vs code, just add the following settings:
55+
``` json
56+
{
57+
"go.lintTool": "golangci-lint",
58+
"go.lintFlags": [
59+
"--fast"
60+
]
61+
}
62+
```
5363
### Unit Tests
5464

5565
```bash
5666
go test
5767
```
5868

59-
### e2e Tests
60-
61-
To run tests against a SQL warehouse, you need to pass a DSN environment variable first:
62-
63-
```
64-
$ DATABRICKS_DSN="databricks://:[email protected]/sql/1.0/endpoints/12345a1b2c3d456f" go test
65-
```
66-
6769
## Issues
6870

6971
If you find any issues, feel free to create an issue or send a pull request directly.
@@ -74,4 +76,4 @@ See [CONTRIBUTING.md](CONTRIBUTING.md)
7476

7577
## License
7678

77-
[Apache 2.0](https://github.com/databricks/databricks-sql-nodejs/blob/master/LICENSE)
79+
[Apache 2.0](https://github.com/databricks/databricks-sql-go/blob/main/LICENSE)

0 commit comments

Comments
 (0)