Changes between Initial Version and Version 11 of Ticket #277

08/23/12 21:28:07 (11 years ago)
Christopher Allan Webber

(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,

    col = Column('collections', Integer)

... 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

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:
-    User, MediaEntry, Tag, MediaTag, MediaComment, MediaFile, FileKeynames,
+    User, MediaEntry, Tag, MediaTag, MediaComment, Collection, CollectionItem, MediaFile, FileKeynames,
  • Again here:
  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(

Overall this looks really promising, and I'm definitely excited about it! :)


  • Ticket #277

    • Property Status NewIn Progress
    • Property Owner changed from Aaron Williamson to Aaron Williamson
    • Property Milestone
    • Property Keywords post-sql added
    • Property Type defectenhancement