Skip to content

Conversation

@minrk
Copy link
Member

@minrk minrk commented Sep 26, 2016

allows storing the test data in postgres via psycopg2.

Even the n=2 case of sqlite and psycopg2 have a lot less in common than I'd like,
so maybe we shouldn't do this and go with sqlalchemy instead.

I thought that dbapi provided some minimal common API, but really the only thing they have in common is a few method names (not even their signatures or argument-formatting conventions are common).

The WIP bit is for actually triggering the tests on Travis, which don't yet run the tests with postgres.

testing creates it
and test psycopg2 on Travis

Requires some abstractions that I don't like, because dbapi providers are not especially similar to each other, apparently, even for the simplest of cases.
@takluyver
Copy link
Member

IIRC NotebookNotary is already intended to be pluggable. Would it be easier to support other databases by subclassing it as e.g. PostgresNotebookNotary, rather than making another pluggable layer for the database?

(The answer may well be no, but I thought it was worth asking the question)

@minrk
Copy link
Member Author

minrk commented Sep 26, 2016

@takluyver that's a good point, and may well turn out better. I set out to do this because I assumed it would be pretty simple (make dbapi constructor configurable and we're done), but I dramatically overestimated the commonality provided by dbapi providers, which is close to zero.

@rgbkrk
Copy link
Member

rgbkrk commented Sep 26, 2016

I'd really prefer not to use SQLAlchemy with Tornado - it's really annoying in production once the workload is big enough. SQLAlchemy (and ORMs in general) is not well suited to our asynchronous style with Tornado.

@minrk
Copy link
Member Author

minrk commented Oct 9, 2016

@rgbkrk yeah, the O, the R, and the M part are really unnecessary and unhelpful for any of our use here. But if we want to support multiple backends, we need some mechanism that abstracts the difference, because apparently there isn't even a tiny subset of a common API or syntax for the extremely basic stuff we want to do.

I also wouldn't be too broken up about sqlalchemy (or similar) synchronous db stuff in our tornado apps, as db operations are very small (storing/looking up signatures is the only one right now), and can be plopped easily enough into a single background thread via ThreadPoolExecutor.

@minrk minrk added this to the no action milestone Nov 30, 2016
@minrk minrk closed this Nov 30, 2016
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.

3 participants