Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sf rebase#1 #159

Open
wants to merge 115 commits into
base: master
Choose a base branch
from
Open

Sf rebase#1 #159

wants to merge 115 commits into from

Conversation

rajsigma
Copy link

Description

Please explain the changes you made here.

Checklist

  • Code compiles correctly
  • Run make fmt to fix inconsistent formats
  • Run make lint to get lint errors and fix all of them
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary

Agam Brahma and others added 30 commits May 10, 2023 15:21
[Maintenance] enable CI tests in our repo (#20)

add README note describing how CI secrets are set up

change yaml to reference environment

remove tests for platforms other than Ubuntu + AWS

disable staticcheck

add encrypted parameters file

add private key

disable tests that fail

[Maintenance] Ignore vendored libs
* add hook to fetch "monitoring" data and query-graph, in goroutines

* add support to fetching monitoring info

* add monitoring threshold

* move to Duration

Lint/Build fixes

nit: Goimports fixes

- [Feature] [monitoring] fetch the query graph for slow queries (#26)

* fetch query graph

* fix

* more fixes

update interface

move monitoring to a goroutine; client decides how long it'll wait (#27)

add MonitoringResultFetcher to fetch monitoring (#48)

- not use qid from SFconnection when fetch monitoring data (#52)
…x. (#44)

* nit: Move "send error on channel" within the `recover` condition.

* Improvement: don't panic _indiscriminately_, have a separate error-type that signals the internal panics we want to re-propagate.

Co-authored-by: Agam Brahma <[email protected]>
- Allow clients to specify DataTypeNull explicitly (#50)

* first cut of explict DataTypeNull

* make lint

* more tests

- Allow client to specify any data type explicitly (#28)

* dataTypeMode should work for all types

* fix test

* allow explicit binaryType declaration even if we're already using binaryType

* silly attempt with *SnowflakeDataType

* switch from *SnowflakeDataType --> SnowflakeDataType

* bindings_test.go passes

* add test for corner case

* fix lint

* fix null-handling tests

* only need connection.CheckNamedValue

* use checked cast instead of switch on type
…45)

lint: fix `ConnectionId`

Fix CI for current master (#47)

* lint: fix `ConnectionId`

* Test fixes for previous change (`41c90a09`)

* nit: tweaks to dsn-test

Co-authored-by: Agam Brahma <[email protected]>

fix var inits (#46)
SIG-18794: Refactor the monitoring fetcher & auto-cancel async (#57)

* SIG-18794: Refactor the monitoring fetcher & auto-cancel async

  * better configuration
  * wait until the monitoring data refers to completed queries

SIG-16907: Error out early when invalid state for multi-statement requests. (#43)

Remove the debug-tracing added earlier.

Co-authored-by: Agam Brahma <[email protected]>

SIG-18794: Make getAsync wait until the query completes or times out. (#59)

* SIG-18794: Make getAsync wait until the query completes or times out.

  * make the getStatus for an async made query block until the ctx
    timeout

SIG-18794: Fix a bug when looping to complete an async query (#60)

* discovered a new error code the needs looping when async,
     enhanced the test
reconcile with origin/master

nit: fix build and gitignore

dead code

Lint fix

nit: fix erroneous deletion

nit: s/uuid/UUID

nit: refactore use of monitoring fetcher for multi-stmt

Refactor out older version of `getMonitoringResult` that didn't have the `endpoint` parameter
Adds a SubmitSync method to the Snowflake driver that directly calls Snowflake's POST /queries/v1/query-request API endpoint synchronously, waiting for up to 45 seconds (the current, fixed timeout) but does not enter the "ping pong" phase of fetching query results for long-running queries. Instead, the caller is responsible for using the query ID to wait for and fetch query results.

This prototype mainly exists right now so that we can test the relevant functionality and compare performance with the fully asynchronous mode of execution.
…t for results to finish without returning result rows (#84)
…ng TokenAccessor (#86)

* Add TokenGetter interface

* Add public constructor for TokenAccessor

* Expose FillMissingConfigParameters

* Add comment

* Add comments

* Fix comment
* add panic

Log whether or not we use the cache here (#95)

* Log whether or not we use the cache here

* Update monitoring.go

* experiment

* Update connection.go

* make status error thing more clear an explicit

* more logging

* Don't assume error type

* add more logging if request fails

* pass qid

* also tag code

* Retry one more time & log response

* log one more thing

* skipCache

* add the proper logging

* retry on bad response

* retry at most 2 times

* lint

* Dont check status before block
Don't cache non success responses and debug the body response
* the result can return just Data field, so we parse a code as an error only if it is set as an error.
Revert "Don't cache non success responses (#109)"

This reverts commit ff7d6b9.
* add logging for sf cache bug

* add log without data to see if prod is seeing this issue
* [Feature] Add support for client log context hooks

* Pin honnef.co/go/stools/cmd/staticcheck dep version

* Add comment
aldld and others added 21 commits August 29, 2023 15:48
* Only cache success responses

* Add another if
* rebase master with head

* Add a nil check for snowflakeRows.Close (#123)

* Only cache successful API responses (#124)

* Only cache success responses

* Add another if

---------

Co-authored-by: Eric Bannatyne <[email protected]>
)

* Add a nil check in snowflake driver heartbeat function

* Update heartbeat.go
…ued in the warehouse (#139)

* proof of concept: materialization more info

* add status to status response
* dont require user & pass for token accessor type

* undo rows commit

* undo connection commit
* Add more visibility in chunk downloader

* Added changes that are on stable branch but not on master

* Added comments to pass lint
…of 2) (#158)

Remove logging sql text from the snowflake driver
@rajsigma rajsigma had a problem deploying to gosnowflake-ci-env August 29, 2023 23:43 — with GitHub Actions Failure
@rajsigma rajsigma had a problem deploying to gosnowflake-ci-env August 30, 2023 00:21 — with GitHub Actions Failure
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.