launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

The proposal to merge lp:~jtv/maas/record-worker-shared-secrets into lp:maas has been updated.

Description changed to:

Sometimes a worker needs to act independently without a server request.  But it gets API credentials and such from server requests.  We need to store the stuff in worker-side variables, but we'd prefer not to write such sensitive information to the worker filesystem.

This branch provides a generic way of maintaining these items.  I hope the docstring on “refresh” explains it well enough that there's no point repeating it here.  We could now, if we wanted, add an API call for workers to say “hey I just started up, can you send me the credentials etc. that I need please?”  It could be synchronous (get results back from API call, call refresh() directly) or asynchronous (ask the server to send a refresh task).  Having it be a celery task leaves that choice wide open.  If we're going to send API credentials, returning them directly from an API call may not be the right choice.  :)  But we can have an unauthenticated request for the server to re-send a refresh task.

One detail worth noting is that in the tests, I don't bother making every call to a task through celery.  Instead I call them directly (this is one of celery's documented selling points, not a hack based on any implementation knowledge) and treat the ability to call the task through celery as an integration matter that I test separately.  I think this keeps the tests more to the point as there's no more need to deal with delay() and the results it returns.  Test failures related to celery integration also become much more informative this way: the it's-a-task test fails but others can still pass — I know because I forgot to add the celery resource to my test initially!

Jeroen

For more details, see:
https://code.launchpad.net/~jtv/maas/record-worker-shared-secrets/+merge/118667
-- 
https://code.launchpad.net/~jtv/maas/record-worker-shared-secrets/+merge/118667
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/record-worker-shared-secrets into lp:maas.