Code review comment for lp://staging/~julian-edwards/maas/api-for-power-query

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Thorough work. I do have a few questions:

.

Half a minute seems generous... Are we really likely to need that long? It's an awfully long time to hold up an app server thread.

And if calling this method is likely to have a noticeable impact on the system, I wouldn't name it as if it were just a getter. Why not call it query_power_state to make it clear that it's “doing things” and not just “returning data”?

.

Not sure it matters but it looks to me as if 503: Service Unavailable was meant to signify that the web service as such is down, not just the resource that's being accessed.

.

What does “Power state is not queryable” mean? Is there anything more that can be said about that kind of error?

review: Approve

« Back to merge proposal