https://dev.arvados.org/https://dev.arvados.org/favicon.ico?15576888422015-11-06T16:24:58ZArvadosArvados - Idea #7711: [Node Manager] Record a node's cloud cost information in the Arvados node record's propertieshttps://dev.arvados.org/issues/7711?journal_id=321462015-11-06T16:24:58ZBrett Smithbrett.smith@curii.com
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/32146/diff?detail_id=31546">diff</a>)</li></ul> Arvados - Idea #7711: [Node Manager] Record a node's cloud cost information in the Arvados node record's propertieshttps://dev.arvados.org/issues/7711?journal_id=321472015-11-06T16:25:26ZBrett Smithbrett.smith@curii.com
<ul><li><strong>Subject</strong> changed from <i>[Node Manager] Record a node's cloud cost information in the Arvados node record's properties</i> to <i>[Node Manager] Record a node's cloud cost information in the Arvados node record's info</i></li></ul> Arvados - Idea #7711: [Node Manager] Record a node's cloud cost information in the Arvados node record's propertieshttps://dev.arvados.org/issues/7711?journal_id=322692015-11-12T02:08:53ZBrett Smithbrett.smith@curii.com
<ul><li><strong>Target version</strong> changed from <i>Arvados Future Sprints</i> to <i>2015-12-02 sprint</i></li></ul> Arvados - Idea #7711: [Node Manager] Record a node's cloud cost information in the Arvados node record's propertieshttps://dev.arvados.org/issues/7711?journal_id=323142015-11-12T15:37:37ZBrett Smithbrett.smith@curii.com
<ul><li><strong>Assigned To</strong> set to <i>Tom Clegg</i></li></ul> Arvados - Idea #7711: [Node Manager] Record a node's cloud cost information in the Arvados node record's propertieshttps://dev.arvados.org/issues/7711?journal_id=323292015-11-12T15:48:39ZTom Cleggtom@curii.com
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/32329/diff?detail_id=31767">diff</a>)</li><li><strong>Subject</strong> changed from <i>[Node Manager] Record a node's cloud cost information in the Arvados node record's info</i> to <i>[Node Manager] Record a node's cloud cost information in the Arvados node record's properties</i></li></ul> Arvados - Idea #7711: [Node Manager] Record a node's cloud cost information in the Arvados node record's propertieshttps://dev.arvados.org/issues/7711?journal_id=325032015-11-17T16:45:37ZTom Cleggtom@curii.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li></ul> Arvados - Idea #7711: [Node Manager] Record a node's cloud cost information in the Arvados node record's propertieshttps://dev.arvados.org/issues/7711?journal_id=325932015-11-18T21:53:02ZTom Cleggtom@curii.com
<ul></ul><p>7711-record-node-price @ <a class="changeset" title="7711: Store cloud node size id and price in properties of Arvados node record." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/d48dda497ae0c4ef85f68f6d48434bca59b84c65">d48dda4</a></p> Arvados - Idea #7711: [Node Manager] Record a node's cloud cost information in the Arvados node record's propertieshttps://dev.arvados.org/issues/7711?journal_id=326492015-11-20T15:02:26ZBrett Smithbrett.smith@curii.com
<ul></ul><p>Reviewing <a class="changeset" title="7711: Store cloud node size id and price in properties of Arvados node record." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/d48dda497ae0c4ef85f68f6d48434bca59b84c65">d48dda4</a>. This is good to merge. My thoughts here are more philosophical, but there's nothing to block a merge.</p>
<ul>
<li>What do you think about rolling this update into the Arvados Node API call we already do just before we create the cloud node, in create_arvados_node/prepare_arvados_node? My feeling on this is that remote API calls are the main opportunities for things to go wrong, and so the fewer of them we do, the better. And it seems like we have all the information we need at the time. But I realize there's a certain information purity in waiting until the cloud node is created.</li>
<li>I see the value in recording the size ID as well, for easier debugging and the use of other components. I worry about this a little bit, because I feel like Node Manager itself should get this information from libcloud whenever possible, and I'm worried a future hacker is going to see this code and use the information in the Arvados record because it's more discoverable. Maybe a comment to this effect in the update code would be nice?</li>
<li>In the places where you added <code>self.make_mocks()</code> in the tests, it looks to me like <code>make_actor()</code> is just buggy. When mocks aren't already made and it gets no arguments, it ends up calling <code>self.make_mocks([None])</code>, which I don't think ever makes sense. If you're feeling up for fixing that (changing the argument from <code>[None]</code> to just <code>None</code>), I think it would be a better long-term change, but no big deal if not.</li>
</ul>
<p>Thanks.</p> Arvados - Idea #7711: [Node Manager] Record a node's cloud cost information in the Arvados node record's propertieshttps://dev.arvados.org/issues/7711?journal_id=326542015-11-20T20:32:54ZTom Cleggtom@curii.com
<ul></ul><p>Brett Smith wrote:</p>
<blockquote>
<p>Reviewing <a class="changeset" title="7711: Store cloud node size id and price in properties of Arvados node record." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/d48dda497ae0c4ef85f68f6d48434bca59b84c65">d48dda4</a>. This is good to merge. My thoughts here are more philosophical, but there's nothing to block a merge.</p>
<ul>
<li>What do you think about rolling this update into the Arvados Node API call we already do just before we create the cloud node, in create_arvados_node/prepare_arvados_node? My feeling on this is that remote API calls are the main opportunities for things to go wrong, and so the fewer of them we do, the better. And it seems like we have all the information we need at the time. But I realize there's a certain information purity in waiting until the cloud node is created.</li>
</ul>
</blockquote>
<p>I started out with that approach, but it was a bit verbose, and I figured we'll soon enough want to add some more information that we don't have until after create_cloud_node() so this seemed better structurally. I've added a comment to that effect, so that opportunity doesn't get optimized away unnoticed.</p>
<blockquote>
<ul>
<li>I see the value in recording the size ID as well, for easier debugging and the use of other components. I worry about this a little bit, because I feel like Node Manager itself should get this information from libcloud whenever possible, and I'm worried a future hacker is going to see this code and use the information in the Arvados record because it's more discoverable. Maybe a comment to this effect in the update code would be nice?</li>
</ul>
</blockquote>
<p>Yes. Added comment.</p>
<blockquote>
<ul>
<li>In the places where you added <code>self.make_mocks()</code> in the tests, it looks to me like <code>make_actor()</code> is just buggy. When mocks aren't already made and it gets no arguments, it ends up calling <code>self.make_mocks([None])</code>, which I don't think ever makes sense. If you're feeling up for fixing that (changing the argument from <code>[None]</code> to just <code>None</code>), I think it would be a better long-term change, but no big deal if not.</li>
</ul>
</blockquote>
<p>Yes, you're right. Fixed.</p>
<p>Now at <a class="changeset" title="7711: Store cloud node size id and price in properties of Arvados node record." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/743341f8a70b126b82ef3293bd9f6c2bc47ec29a">743341f</a></p> Arvados - Idea #7711: [Node Manager] Record a node's cloud cost information in the Arvados node record's propertieshttps://dev.arvados.org/issues/7711?journal_id=326572015-11-20T20:35:10ZTom Cleggtom@curii.com
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li></ul><p>Applied in changeset arvados|commit:055ac1cc91121d3f27405d4f45455a8de5a0e57e.</p>