Code review comment for lp:~jseutter/landscape-client/srv-autodiscover

Hi Thomas,

1,2,3,4,5,6,7 should be fixed.

> [8]
> + def handle_result(result):
> + if result is None:
> + logging.warning("Autodiscovery failed. Reverting to previous
> "
> + "settings.")
> + return result
> +
> + def do_rest(result):
> + if result:
> + self.url = "http://%s/ping" % result
>
> This is slightly strange, because you encode the logic of autodiscovery
> returning None in both places. You should return the URL in handle_result no
> matter what.

Yeah, the code is a bit tortured here. I tried cleaning it up but without much effect. The problem is that a deferred is created only when autodiscovery is enabled. When autodiscovery is disabled the code calls do_rest(None), so checking for None is done in both places. Were you thinking of a better way to do the same thing?

> [9] It'd be nice to cook an DNS server script to answer those kind of
> requests, so that we can test locally easily.

I added a small script to the dev/ directory that developers can run on their local system. Using it will also require editing /etc/resolv.conf manually to point the dns client to this server.