Code review comment for lp:~lifeless/launchpad/private-librarian

Revision history for this message
Robert Collins (lifeless) wrote :

I used the session DB because this stuff is transient : memcached might be an answer too but the token *system* has to be available or private file downloads will fail, and memcached is currently (AIUI) a 'when its there use it' system - which is fine. Replicating the tokens isn't terribly useful given we're just going to delete them again, but all of that is a bit beside the point : yes, we do need to commit changes in get requests :).

Will move the definitions to launchpad_session.sql.

We could store the expiry time, but I figured we'd make the lifetime a config option and thus work from the creation point rather than expiry.

I figured we might want reporting on stuff in the table - thats why I used url, token, rather than a single column. Easy to change if you have a strong opinion any which way - I don't.

I can't answer the url/path segment question yet : once its fully hooked up it should be obvious what is easiest. The current schema is what was easiest based on the attributes of the objects I am using (LibraryFileAlias etc).

We could make the librarian offer a redirect *back* to launchpad for expired tokens, rather than a go-away message - adding the source canonical_url to the table would permit that trivially.

Re: exceptions for anonymous users calling TimeLimitedToken.allocate - I can add that in, is there some existing similar safety need I can reference?

-Rob

« Back to merge proposal