"Show progress" should be implied

[fastolfe]

I shouldn't have to:

(a) open a cert (b) and click on 'Show Progress'

to see my progress on certificates. The default cert view should include my progress, and when looking at the list view (global or my certs), I should see my progress for each cert as a column in the table.

[ayust]

This would seem to assume that you only have one character. How would you envision this working for users with multiple characters?

[fastolfe]

It seems like a drop-down at the top of each page with the list of characters in it would work. That would make it no more difficult to flip between them than it is today.

[ayust]

Yeah, that's true.

On the back-end front, it would mean running datastore queries for the skills which are part of each certificate, though caching could potentially mitigate that.

[fastolfe]

I'm interested in implementing this. I'm somewhat new to GitHub (and AppEngine for that matter), though. The approach I'm thinking of taking:

1. Refactor main.py into api_keys.py, skill_tree.py, certs.py, with the new files not having any WSGI handler bits in them (they'd just have functions for accessing/managing their respective models, fetching cert completion, etc.)
2. Add a new CurrentCharacter class to models.py that has an owner (UserProperty) and char_id (or a ReferenceProperty to a Character?)
3. Create a character selection drop-down that auto-submits a character change, apply it to the relevant pages
4. Add completion stats to the pages that list certificates

Does that sound reasonable?

[ayust]

I'd like to hear more about your thoughts on how the number of datastore queries required for rendering pages will change as part of (4) before you get too far into implementing it. That said, I'm generally okay with the idea of (1) through (3).

[fastolfe]

Still digging through the code a bit, but I think today it works like this:

Show certificate page:

• Retrieve cert object from data store
• Retrieve API keys/characters from data store for current user
• Retrieve skill tree from data store (memcached)
• Retrieve character sheet from EVE (may be cached by evelink)

So, 2 or 3 datastore hits, depending on if the skill tree is cached, and 1 hit to EVE.

List certificates:

• Retrieve cert objects from data store

So without any additional caching, a new list view would be essentially the same as the show certificate view, except that it's iterating over all certificates (still should be one datastore hit?). A typical page hit (skill tree cached) will add one additional data store hit (doubling the number of hits for that page).

It's likely that some people won't visit the 'show certificate' page as often if they can get the summary from the list view.

Opportunities for caching:

• Cache the API keys/characters for the current user. This should be relatively stable and we can invalidate it when the user makes changes or refreshes the API key.
• Possibly cache the character sheet from EVE, but it looks like the evelink stuff does caching automatically?

[ayust]

Under the current data models, the skills required for a particular cert are in separate linked models, so it's not just retrieving a single cert object from a datastore, it's retrieving a cert object and then retrieving the relevant skills for that cert object. Doing that for multiple certs would make the number of datastore queries increase linearly along with the number of certs (since AppEngine's "in" queries are actually just parallel datastore queries).

[fastolfe]

Ah, of course, thanks. It looks like it is possible to make one pass through Certification.all(), and pull out the required_skills keys (without dereferencing them). We can then follow up with a single datastore invocation against RequiredSkill using the set of keys, and fully populate the Certification instances with the returned objects. I found this technique at http://blog.notdot.net/2010/01/ReferenceProperty-prefetching-in-App-Engine. What do you think?

[ayust]

That seems like a reasonable way to do it while keeping the query number sane. I'm okay with you moving ahead; let me know when you have some code for me to review or if you have any other questions.

[ayust]

One other small note - if the EVE API request for the character skills times out, it'd be nice to have the system fail gracefully and still display the skill list, just without progress.

[fastolfe]

OK. I'm new to the Github workflow. Should I do 'pull' requests for each step from my list above, or wait until I have it all done and send it as one big batch? Should I try to funnel this into some code review tool?

[ayust]

I would suggest doing (1) first in a branch, and submitting it as a pull request for code review, then building (2-4) as another branch built on top of the branch for (1) and with a separate pull request and code review.

The reasoning being that (1) is mostly independent, and will give you a chance to get acquainted the code and get some review feedback; (2-4) are fairly closely linked and probably shouldn't be rolled out piecemeal.