(this review below mostly done from a car, sorry for annoyingness... the questions still apply:)
This is a very brief review without being able to test it. I hope to do an actually-test-it review shortly. Here's from a code skim though:
- What's this "association_proxy"?
collections_helper = relationship("CollectionItem",
cascade="all, delete-orphan"
)
collections = association_proxy("collections_helper", "in_collection")
Relatedly, I'm confused by:
@RegisterMigration(4, MIGRATIONS)
def add_mediaentry_collections(db_conn):
metadata = MetaData(bind=db_conn.bind)
media_entry = Table('core__media_entries', metadata, autoload=True,
autoload_with=db_conn.bind)
col = Column('collections', Integer)
col.create(media_entry)
db_conn.commit()
... why an integer?
I guess I wonder why each collection doesn't have a foreignkey to the MediaEntry?
Admittedly I don't know anything about association_proxy and am writing this when on my laptop without intarwebs access. :)
... okay, I've read more, and http://docs.sqlalchemy.org/en/rel_0_7/orm/extensions/associationproxy.html
does seem like a really cool interface. Is the coolness of that interface the primary reason for the proxy table though? I somewhat wonder if you couldn't get something similar with general sqlalchemy relationships assuming it linked directly to the table.
See below for me wondering if this has anticipated extra extensibility beyond MediaEntry stuff...
- I notice there's a Collection and CollectionItem table, which is pretty awesome. But I also notice there's a collections mixin. Not that it's wrong, just trying to understand... why the mixin? Do we anticipate more things than these two tables to use this functionality?
Premature abstraction might be right or wrong, not sure. Would be useful to know your thinking.
- Style nitpick... should wrap at 80 characters:
MODELS = [
- User, MediaEntry, Tag, MediaTag, MediaComment, MediaFile, FileKeynames,
+ User, MediaEntry, Tag, MediaTag, MediaComment, Collection, CollectionItem, MediaFile, FileKeynames,
MediaAttachmentFile]
form.collection.query = request.db.Collection.query.filter(filt).order_by(request.db.Collection.title)
could be done as:
form.collection.query = request.db.Collection.query.filter(
filt).order_by(request.db.Collection.title)
Overall this looks really promising, and I'm definitely excited about it! :)
