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
Adding support for SQLite #56
Conversation
@ak-gupta I think for the failing tests we can change the dev requirements files to the following:
Just set the connectors to be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think tweaking the requirements file will help the tests pass if you have some time to push that.
Thanks for the PR too!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @jborchma You ok with this change?
Oh, do I need to increment the version before merging to master? |
No need. I'll take care of that once we are ready to cut a new release. |
Ok great, just wanted to make sure! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Just a small question.
@@ -110,7 +110,7 @@ def _disconnect(self): | |||
else: | |||
logger.info("No connection to close") | |||
|
|||
def execute(self, sql, commit=True, params=None): | |||
def execute(self, sql, commit=True, params=()): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am curious about this change here. Is this to align with some convention for SQLite?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, when I tried to use Database
with params=None
it didn't work
In this pull request I have added support for SQLite as well as an additional feature for Snowflake users. If the user supplies
schema
in their credentials,locopy.snowflake.Snowflake
will runUSE SCHEMA {schema}
for the user.Fixes #50
Fixes #55