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

Support for SQLite3 #50

Closed
ak-gupta opened this issue Jul 17, 2019 · 5 comments · Fixed by #56
Closed

Support for SQLite3 #50

ak-gupta opened this issue Jul 17, 2019 · 5 comments · Fixed by #56
Assignees
Labels
enhancement New feature or request

Comments

@ak-gupta
Copy link
Contributor

I tried to connect to a local SQLite3 database using Locopy and it threw the following error:

ValueError: parameters are of unsupported type

After doing some digging, it looks like the error is being thrown by the default argument in locopy.database.Database.execute() for params. Once I started using params=(), it started working.

e.g.

with locopy.Database(dbapi=sqlite3, database=':memory:') as cmd:
  cmd.execute('''CREATE TABLE stocks (date text, qty real)''', params=())

I tested params=() with locopy.snowflake.Snowflake as well and it worked.

@jborchma
Copy link
Collaborator

To be fair, there are already a lot of SQLite packages out there. I wonder if this should be really the scope of locopy.

@fdosani
Copy link
Member

fdosani commented Jul 23, 2019

I'm ok with this if it extends support to other DBAPI 2.0 adapters. I don't think we'd add loading / unloading functionality but just to use the execute should be fine especially if it is just tweaking params. @ak-gupta Did you want to propose a PR with tests for this?

@ak-gupta
Copy link
Contributor Author

@jborchma Yeah I agree with your point. I guess I was more focused on whether or not the current default params=None is correct given that it has no impact on Snowflake queries and it allows users to query SQLite databases.

@fdosani Yes I was thinking the same; it's probably out of the package scope to add a new Database subclass. I can work on a PR

@fdosani
Copy link
Member

fdosani commented Aug 19, 2019

@ak-gupta any traction on this feature? No pressure, just wondering if you needed a hand

@fdosani fdosani added the enhancement New feature or request label Aug 22, 2019
@ak-gupta
Copy link
Contributor Author

Sorry, I didn't have a chance to work on this until recently. I've made the changes and it looks like the tests are running nicely! I'll submit a PR once I've done another sweep.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants