Code review comment for lp:~allenap/maas/fix-makefile-write-dhcp-stuff

Revision history for this message
Raphaƫl Badin (rvb) wrote :

Looks good. +15/-66 without loosing functionality, that's nice.

Maybe we can wait for Julian to clarify things about [0] but I'll leave it up to you because, in my opinion, this is all good.

[0]

> Removes the write_dhcp_config script entirely. It's not necessary -
> use bin/py -m provisioningserver/dhcp/writer instead - and is
> misleading because packages cannot use it anyway as it is generated
> by buildout.

I agree with you. Since Julian is away I can only speculate here but this script can only be a development convenience and if it's the case then using the command as you suggest is, I think, good enough.

So far, when we had the need to expose a command, the usual way to do that has been to create a Django command. Now, unless I'm mistaken, this is part of the provisioning server because it will be run by the workers. We haven't talked about this but the workers will probably need to install a specific maas package (something like 'maas-worker') rather that the server 'maas' package and this worker package will only contain the worker code (i.e. the module 'provisioningserver'). This implies that if we want to ship scripts in this worker package we will have to come up with a new strategy because the Django MAAS application (and thus CLI) won't be installed there.

review: Approve

« Back to merge proposal