Hi thanks for very thorough response. I would love to help with writing unit tests for this. I already started studying the testing architecture of SQLAlchemy. Any tips you can give me there? :)
Also I read from the testing docs that in general SA tries to adhere to PEP8 standards. I could make some pull requests for making the code more pep8 compliant also. - Konsta On Monday, April 14, 2014 9:53:22 PM UTC+3, Michael Bayer wrote: > > > On Apr 14, 2014, at 2:26 AM, Konsta Vesterinen > <[email protected]<javascript:>> > wrote: > > Hi, thanks for quick reply Mike. Let's say the user has three models as > defined in the SA joined table inheritance > docs<http://docs.sqlalchemy.org/en/rel_0_9/orm/inheritance.html#joined-table-inheritance>. > > Now the user applies versioning to these models. One way I could model > these relationships is only apply the versions relationships to all 'leaf > classes' in other words classes that are the leaf nodes in the join table > inheritance tree. > > > that paragraph might have the first clue - “the versions relationships” - > so you’re adding rows to a versioning table, then linking it to the parent > with relationship(). OK, that’s not how I’ve done that before, I just had > a new class like MyClass.MyClassHistory. But yes, I see how that is a good > idea, an object has a .versions collection. Sure OK. > > > So in this example I could apply those relationships to Manager and > Engineer classes. This means the Employee class wouldn't have versions > relationship. I think this is not a problem IF the user has set > with_polymorphic=True on the Employee class since then instances of > Employee class are never returned by SA queries. > > > However what should happen if user doesn't have with_polymorphic as True > and he tries to access versions relation of Employee model? > > > My first impression, is that the version classes should be polymorphic. > Since your plugin is already making decisions as to how these tables look > and how they are built out, a polymorphic discriminator column should be > assembled as well. If i have Engineer.versions, I link to the same > .versions that’s on Employee.versions, the rows it locates will all have > “engineer” as the discriminator and I get EngineerHistory objects back. > But see next paragraph. > > > The other way of modeling the versions relationship would be apply it to > Employee model and set with_polymorphic=True on the EmployeeVersion class. > This would work on all cases but lead to somewhat inefficient queries in > some cases (if there are lots of classes in the inheritance tree, these > classes are always joined in the queries). > > > OK,you’ve already thought of that, and thinking what the inefficient case > is, OK that’s because if you do Engineer.versions, it only queries Employee > table to start with because it doesn’t know to query for EngineerHistory, > OK. > > So here’s a patch: > https://bitbucket.org/zzzeek/sqlalchemy/issue/3022/propagate-false-for-relationship, > > but here’s the problem, it has no tests. it also uncovered some edge > cases in concrete mappings that have to be tested. If you have interest > in learning to write tests for SQLAlchemy itself, let me know. > > A test script is present in the description of that issue which works. > > This issue should be tracked for the next 0.9.x release that’s possible. > Help would be great! > > > > > -- You received this message because you are subscribed to the Google Groups "sqlalchemy" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/sqlalchemy. For more options, visit https://groups.google.com/d/optout.
