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

cleaning up the code to manage connections #282

Closed
edublancas opened this issue Mar 21, 2023 · 0 comments · Fixed by #515
Closed

cleaning up the code to manage connections #282

edublancas opened this issue Mar 21, 2023 · 0 comments · Fixed by #515

Comments

@edublancas
Copy link

We have three types of "database connection" objects.

In our codebase, we manage connections with a Connection object, this is required for the %%sql magic to work. Internally, this connection object has a sqlalchemy connection. Finally, in some tests (or examples) we use the native sqlite3 or duckdb drivers for creating sample data.

This is creating inconsistency in our code and confusion for contributors who might encounter errors when they expect a conn argument to be of a particular type but turns out to be from a different type. So we should ensure we have proper guidelines about when to use each.

  1. Connection should be exclusively used to manage database connections on the user's behalf and to obtain the current SQLAlchemy connection (example)
  2. Functions that expect a conn (sometimes named con) input variable should only use SQLAlchemy connections; this must be documented and we should test that the functions work with SQLAlchemy connections
  3. When creating data for tests, we should use sqlalchemy.create_engine and discourage the use if the native driver functions (e.g., sqlite3.connect or duckdb.connect) to ensure consistency. This must also be documented in our developer guide
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 a pull request may close this issue.

1 participant