On Wed, Jun 26, 2013 at 8:01 PM, Jed Smith <[email protected]> wrote:
> This looks great, Alex! I haven't studied intently, but I have a
> couple quick comments:
>
> Line 16: Yes, I think it should be a parameter on create_volume. That
> makes the implementation less awkward if a provider does not support
> snapshotting.
>
> Line 70: What does node= do on detach()? Is it a sanity check in case
> your idea of state is wrong, or supporting attachment to multiple
> nodes at a time?
>
>
Both!
> I think some of the naming is wordy. As one example, you'll end up as
> a client with something like:
>
> node = ...
> volume = ...
> result = driver.node_attach_volume(node, volume, '/dev/xvdq')
>
>
This was largely inspired by the load balancer code, I imagine in practice
people will do `node.attach_volume(volume, '/dev/dfdasf')` assuming we're
ok with that type of cross-system API?
> To steal a Zed Shaw term, feels a bit like a feature chant ("node Node
> NODE"), but it fits with the rest of the APIs in my eyes, so probably
> a broader concern than just this design.
>
> --
> Jed Smith
> [email protected]
>
--
"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084