Code review comment for lp:~dylanmccall/harvest/harvest-dylan-m

Revision history for this message
Dylan McCall (dylanmccall) wrote :

I did some pondering and poking, and I'm further convinced to change my use of accessors. Turns out the most Pythonic way is to use plain instance variables and implement property() as appropriate, which lets us specify our own getters and setters for those variables (or just a getter). That Java course nearly corrupted me!

There is a bit of divergence in filters.py in the gsoc-client-stuff branch, so I'm a little reluctant to make that change in this branch. (Merge conflicts — even simple ones — invariably give me headaches). Are you okay if I do it in a new branch from gsoc-client-stuff?

> I was under the impression that we wouldn't have to have all
> opportunities and source packages in memory if the query just asked
> for a specific subset. Maybe I'm wrong. I'll go and find out.

I mean to say that all the source packages which have been met by the package filtergroup are examined in a similar way. It isn't an extensive thing, but we end up accessing the queryset, which gives us a list of all the references it has found, both hidden and visible packages. (The opportunities remain a queryset until the template asks for them). I should move that to be smarter about hidden packages, only storing their count via a query.

With gsoc-client-stuff, I'm working on asynchronously loading results, package info and hidden packages. This means implementing new views on the same query data as before. So, that's why I am a little picky about having one object to manage all that; all these views can just talk to the same thing in a somewhat balanced way. Having said that, it's mostly a stop-gap and if all goes well it should be really easy to yank it out for something quicker and more cool once the dust is settled. To be honest I'm not sure what the final result is going to look like in views.py, so I am reluctant to devote too much to a specific approach yet ;)

« Back to merge proposal