On 5 May 2011 09:17, Stuart Bishop <stuart.bis...@canonical.com> wrote: > On Wed, May 4, 2011 at 10:32 PM, Gavin Panella > <gavin.pane...@canonical.com> wrote: [...] >> Instead we're learning to write this like: >> >> owners = bulk.load_related(Person, lots_of_bugs, "ownerID") > > I suspect this should be a Storm feature. Something like > Bug.owner.load(lots_of_bugs)
That would be neat. +1 Want to write it? :) >> One approach to improving this situation could be to forbid (with >> code) the use of Reference properties that are not already cached (in >> the store cache; this is nothing to do with @cachedproperty). > > Rather than forbid, we could warnings.warn(). This will break tests, > but continue to work live for any untested code paths that sneak > through. I've been thinking that we could use this blocking in two modes: - A warnings mode for development where we can see what's still using non-cached References (with a call stack). We could then either eager load, or annotate/context-manage when it's okay to deferenece. - When the warnings are silenced the mode can be changed to raise an exception instead, to fence off that section of code so that it can't regress. Interactions with GenerationalCache - see my message to Jeroen from earlier - might scupper this idea; i.e. the warning mode might be as far as we can go. Or, for production alone, we could select (via config) the warnings-only mode, and keep exceptions-mode for development only. > >> Doing this everywhere is not going to fly, at least right now - >> there's probably way too much code that would break if we did this - >> and sometimes we would want to permit their use - when we're just >> looking up a single object for example. > > Adding a ratchet might help for the broken tests. ie. only raise that > warning the 10th time we dereference, resetting the counter in > DatabaseLayer.testTearDown(). We often only create one or two entries for a collection for tests, so a count may never get high enough in the test suite. Test set-up would also contribute to a count, possibly causing false warnings. [...] > Enforcing this will catch issues, but I can't see how to enforce it > without also requiring the bulk load syntax be used for accessing a > single item. I suspect this would be an awful lot of fallout in the > current code base, and a lot of typing to fix as I don't think we can > use a script to migrate the code. The Reference.unblocked() context manager is for accessing a single item. Gavin. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-dev Post to : launchpad-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-dev More help : https://help.launchpad.net/ListHelp
