<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Feb 10, 2015 at 6:13 PM, William Reade <span dir="ltr"><<a href="mailto:william.reade@canonical.com" target="_blank">william.reade@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I think that NotAssigned is *too* generic. UnitNotAssignedToMachineErr<br>
is one thing, and VolumeNotAssignedToStorageInstance is another; if we<br></blockquote><div><br></div><div>Okay, fair point; I saw "IsCodeNotAssigned" in apiserver/common and got</div><div>a bit carried away. That really should be "IsCodeUnitNotAssigned" then.</div><div>I'll put the error back in state and add a new, volume-specific one.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
use the same error type to represent those distinct situations, we<br>
really ought to include a bunch more mechanism for finding out<br>
*precisely* what should have been assigned to what (because<br>
refactoring happens, and methods start returning the same generic<br>
error for new reasons, potentially breaking client code that makes<br>
assumptions about what that generic error type implies. See<br>
worker/uniter/filter/filter.go and search for ErrTerminateAgent for a<br>
particularly hair-raising example...)<br>
<br>
I see what you're saying re apiserver/common, but I wouldn't really<br>
characterise it as "poking into" state: apiserver is generally<br>
implemented in terms of state, so that's a perfectly reasonable<br>
dependency IMO. For exactly the reasons stated re generic error types<br>
in code, the generic error codes in the api are the scary bits I now<br>
wish we'd done differently...<br>
<br>
Cheers<br>
William<br>
<div class="HOEnZb"><div class="h5"><br>
On Tue, Feb 10, 2015 at 10:55 AM, Andrew Wilkins<br>
<<a href="mailto:andrew.wilkins@canonical.com">andrew.wilkins@canonical.com</a>> wrote:<br>
> Hi all,<br>
><br>
> Ian raised a good point in <a href="http://reviews.vapour.ws/r/885/" target="_blank">http://reviews.vapour.ws/r/885/</a> (caution advised,<br>
> may cause eyebleed) about a change I made to errors:<br>
> <a href="https://github.com/juju/errors/pull/17" target="_blank">https://github.com/juju/errors/pull/17</a><br>
><br>
> The NotAssigned error, which previously was only raised by<br>
> state.Unit.AssignedMachineId, is now also raised by<br>
> state.Volume.StorageInstance. I removed the error from the state package so<br>
> we didn't have apiserver/common poking its head in there, and moved it over<br>
> to the juju/errors package.<br>
><br>
> The issue Ian raised is that juju/errors should contain relatively generic<br>
> error types, and we should keep domain-specific things elsewhere. Examples<br>
> are NotAssigned, and NotProvisioned. We're thinking of moving these<br>
> domain-specific error types into a new package, juju/juju/errors. What do<br>
> you think about this?<br>
><br>
> Cheers,<br>
> Andrew<br>
><br>
</div></div><span class="HOEnZb"><font color="#888888">> --<br>
> Juju-dev mailing list<br>
> <a href="mailto:Juju-dev@lists.ubuntu.com">Juju-dev@lists.ubuntu.com</a><br>
> Modify settings or unsubscribe at:<br>
> <a href="https://lists.ubuntu.com/mailman/listinfo/juju-dev" target="_blank">https://lists.ubuntu.com/mailman/listinfo/juju-dev</a><br>
><br>
</font></span></blockquote></div><br></div></div>