It'd be nice if there was a way to query stores for their databases

Bug #506536 reported by Guilherme Salgado
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Storm
Fix Released
Undecided
Guilherme Salgado

Bug Description

In Launchpad we have a custom Database class which stores the dsn that was passed to psycopg2.connect(), which we need to know in some cases. The problem is that to get to our Database from a store we need to go through private attributes (store._connection._database), so it'd be really nice if Store had an extra attribute that'd return its database.

Since the database is passed in when a new Store object is created, we could either store that as an instance variable or make it a @property that returns self._connection._database.

Related branches

Revision history for this message
Jamu Kakar (jkakar) wrote :

We talked about it and had some different opinions:

<salgado> jkakar, around? did you see my question earlier about adding a 'database' @property to Store?
<therve> salgado: what about getUtility(IZStorm).get_default_uris() ?
<salgado> therve, we don't use set_default_uri() to populate the default uris -- we always pass the URI to IZStorm.get()
<therve> ah, sorry
 salgado: is the uri stored anywhere on the database though?
<salgado> and I was confused when I said I need the uri
 what I need is something else that is stored in our custom Database class
<therve> ah :)
<salgado> we store the dsn that is passed to psycopg2.connect()
 that's actually stored by Postgres too
<therve> yeah but the dsn is private
<salgado> right, but since we already have a custom Database class, we can create a public version of it there. if only we didn't have to go store._connection._database to get to our database. :)
<therve> it seems kind of weird to me
 you can maintain a mapping outside of storm to keep track of that
<salgado> why is it weird? we already pass a Database object when creating new storm instances, so why not allow the store to be asked about its database?
<therve> I just gave my opinion :)
<salgado> I could maintain a mapping myself, but it'd be a lot easier if storm had that facility. would you be against such a change even if one of your dear users were to do all the heavy lifting? ;)
<therve> the thing is that the default database objects in Storm don't provide anything
 but it doesn't seem like a terrible idea, I'm just saying :)
<salgado> yeah, that's a good point, but it'd be reeeeeaaaaaly nice if storm had that. guess I'll describe my use case in a bug and hope for the best
<jkakar> I think it makes sense.
 I can imagine storing things like a schema generator on the database object.
 Then you could do things like if no_schema(): store.database.schema.generate(store)
<salgado> jkakar, super! would it be ok to just save the database that's passed in to __init__()? (https://bugs.edge.launchpad.net/storm/+bug/506536)
<mup> Bug #506536: It'd be nice if there was a way to query stores for their databases <Storm:New> <https://launchpad.net/bugs/506536>
<jkakar> salgado: That's what I was thinking. That way you can specialize it, like you have, to do useful things.
 salgado: It certainly feels like a very natural place for a schema and read-only status and other database metadata that your application might care about.
<salgado> indeed

Changed in storm:
assignee: nobody → Guilherme Salgado (salgado)
status: New → Fix Committed
Changed in storm:
milestone: none → 0.17
Changed in storm:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.