Code review comment for lp://staging/~allenap/maas/rpc-get-secrets

Revision history for this message
Julian Edwards (julian-edwards) wrote :

On Wednesday 10 Sep 2014 23:20:44 you wrote:
> The first and last of those could be rewritten as RPC calls as a
> straight port. The middle one is the problem: it downloads many
> megabytes (of XML) in a single call. Rewriting that to work over RPC
> would mean inventing a mechanism for packing and unpacking the data over
> the wire. That's square-nail-round-hammer territory, at least for now
> when we'd have to rush a design and implementation.
>
> This code works as it is right now. I can port the Celery tasks that
> trigger it to RPC calls, but I think we should leave this code well
> alone for now. It really isn't doing us any harm.

Fair enough, it sounds like we're better off with the API then.

> As discussed elsewhere, the process pipe (aka `pipefork` and
> `objectfork`) needs to be run from a non-reactor thread. I'll add a
> guard to prevent it from being used in the reactor thread.

Tiptop! Thanks.

> I can live with passing the API parameters per-task rather than having a
> GetSecrets call.

The more I think about it the more I like it. I indicates to the casual
viewer that this RPC will be using the API to return results, and that is a
special situation.

Cheers!

« Back to merge proposal