Use mongokit's "Dot Notation"

[Elrond]

mongokit supports accessing fields not only as doc['field'] but also as doc.field.

• This might look more pythonic.
• This will ease a possible transition to sqlalchemy (as this it the sqlalchemy way)
• Docs: http://namlook.github.com/mongokit/tutorial.html#dot-notation Main questions:
• Switch it on now or in the sql-migration branch?
• If switching it on now, should all accesses be converted to the new syntax?

[Christopher Allan Webber]

Assuming we're doing the SQL migration, we should do this move before rather than later.

[Christopher Allan Webber]

<Elrond> paroneayea - The question is somewhat "Would doing it now hurt in any
         way, no matter what we decide on sql?" ;)  [08:29]
<paroneayea> Elrond: I think it would be pretty harmless!  [08:30]
<paroneayea> and could be potentially helpful
<paroneayea> I'll say as such in the ticket :)

It's also much more pythonic looking. This move gets a thumbs up from me.

[Elrond]

• Migrations use pymongo: No Dot-Notation there!
• new_obj = db.MediaEntry(), new_obj._id = ObjectId() doesn't seem to work as needed. I have no idea, what's actually going wrong.

So I vote for switching on Dot Notation just now and then slowly migrate accesses over.

[Elrond]

1. Enabling the whole thing (really simple, three lines)
2. Convert all Users.* fields over to the dot-notation. (~900 lines diff, 141 lines changed)
3. unit tests pass!
4. I'm stopping here.
5. I'm tempted to push (1) alone just today...
6. I have no idea, what might have broken.
7. I have no idea, what to do with (2)

[Christopher Allan Webber]

If things are working, I say pushing what you have into master right now is a good idea! We can keep this bug open to get the other models, too.

[Elrond]

pushing stuff¶

Okay, pushed (1) (enabling dot notation). Let's see, if this breaks somethign for someone in the next 24 hours. I will push ['_id'] -> ._id tomorrow (it's the biggest part) and let it be there for 48 hours. Then I'll push all the remaining Users fields.

About ._id

About setting ._id, here's a failing simple test case:

test_user = mg_globals.database.User()
test_user._id = u"some_id"
assert_equal(test_user['_id'], u"some_id")

I don't know, if this is a mongokit bug or feature. I haven't checked mongokit's docs on this either.

Note: The following works, which is good!

test_user = mg_globals.database.User()
test_user['_id'] = u"some_id"
assert_equal(test_user._id, u"some_id")

So basicly, we should not assign to ._id for now or create some descriptor trickery to help this case. Or create the descriptor trickery to help us move to something we want to have.

[Elrond]

Okay, here is a snippet of code, which might help with the _id thing and help us migrate to a better named primary key column:

class MongoPK(object):
        def __get__(self, instance, cls):
                return instance['_id']
        def __set__(self, instance, val):
                instance['_id'] = val
        def __delete__(self, instance):
                del instance['_id']

class C(dict):
        pk = MongoPK()

[Christopher Allan Webber]

The primary key stuff might help a small amount, but I think we shouldn't worry about it too much. Whatever involves primary keys will probably have to change anyway, so probably not worth spending too much time on. I think this bug is mostly useful in that it helps tackle some low hanging fruit that would be useful for the ->SQL conversion, if we go that route.

[Elrond]

._id accesses are a lot of the accesses in the code. And not all are really that specific, they're just to use "some primary key like thingy". So not much will need to be changed there.

If we have an sql(alchemy) schema with primary keys, we can change the accesses before going the sql branch.

[Elrond]

I will try to get this done mostly after 0.2.0.

[Elrond]

Okay, as I'm out of town for a bit, I've pushed my current work into a personal branch: sql/dot-notation

Please (review and) merge after 0.2.0.

It doesn't seem to break anything for me. I don't want to break 0.2.0 just that lately. Let's break it afterwards, so that there is plenty of time to fix it.

After merging, don't close this bug, set it back to "In Progress", as I haven't converted all stuff over.

[Elrond]

Okay, merged.

[Christopher Allan Webber]

I've only vaguely looked at this, but things have been working for the last month and I think it's safe to say that this was a good idea transitionally. Closed!

[Will Kahn-Greene]

The original url for this bug was ​http://bugs.foocorp.net/issues/660 .
Relations:
#379: blocks