Skip to content

Refactoring and cleanup in connection-related code - #149

Merged
nithinkdb merged 5 commits into
mainfrom
connection-cleanup
Jun 13, 2023
Merged

Refactoring and cleanup in connection-related code#149
nithinkdb merged 5 commits into
mainfrom
connection-cleanup

Conversation

@kravets-levko

@kravets-levkokravets-levko commented Jun 13, 2023

Copy link
Copy Markdown
Contributor

This PR fixes some things that popped out while I worked on #147. To keep original PR clean, I decided to extract those changes here:

  • Remove redundant and not-really-needed NoSaslAuthentication
  • Improve types: make HttpTransport more type-safe (since we use Thrift only over HTTP, we don't need HttpTransport to be so generic), fix classes that used ITransport/HttpTransport
  • Make PlainHttpAuthentication update headers instead of completely replace
  • Pass User-Agent header through connection object - previously we did it through auth provider because PlainHttpAuthentication was replacing headers and headers provided by HttpConnection were just ignored

May be easier to review commit by commit

…lAuthentication
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
… code
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
@kravets-levkokravets-levko changed the title Refactoring and cleanup in conection-related codeRefactoring and cleanup in connection-related codeJun 13, 2023

@nithinkdbnithinkdb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@nithinkdb
nithinkdb merged commit 0a2bdb4 into mainJun 13, 2023
@kravets-levko
kravets-levko deleted the connection-cleanup branch June 13, 2023 17:29
nithinkdb pushed a commit to nithinkdb/databricks-sql-nodejs that referenced this pull request Aug 21, 2023
* Cleanup DBSQLClient code; remove redundant and no longer needed NoSaslAuthentication
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
* Make HttpTransport and related types more type-safe, remove redundant code
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
* Remove some leftover code
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
* Update tests
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
* Pass User-Agent through HttpConnection instead of auth provider
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
---------
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
Sign up for freeto 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.

2 participants

@kravets-levko@nithinkdb