<div dir="ltr"><div><div><div><div><div><div>Hi, my opinion on this:<br><br></div>Yes, it is technically possible to implement this feature using only tasks. This may require adding new field to tasks to distinguish whether task was run for saved or unsaved changes. But I'm against this approach because:<br><br></div>1) It will require handling more than 1 task of a single type which automatically leads to increase of complexity of the code. We will need 2 tasks in the following case: there are 1 task for unsaved data and 1 for saved. We need to show result of the first task on the network tab and use status of the second task to determine whether the network check was performed.<br><br></div>2) We have 2 similar tasks: for deploying a cluster and settings up a release. Both cluster and release models have "status" field which represent status of these entities so we don't perform complex checks with tasks. So I think the same approach should be used for network verification status.<br><br></div>As for tasks deletion, there are 2 reasons for this:<br><br></div>1) If we don't delete old tasks, it increases the traffic between backend and UI. There are still no way to fetch the latest task or 2 latest tasks using our API.<br><br></div>2) We delete tasks manually when their results are not needed anymore or become invalid. For example, when user adds another node, we remove network check task as its result is not valid anymore. Yet another example - when user clicks X button on the message with deployment result, we remove this task so it won't be shown anymore. If you want us not to delete these tasks, please provide us with another way to cover these cases.<br></div><div class="gmail_extra"><br><div class="gmail_quote">2015-02-09 15:51 GMT+03:00 Przemyslaw Kaminski <span dir="ltr"><<a href="mailto:pkaminski@mirantis.com" target="_blank">pkaminski@mirantis.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
On 02/09/2015 01:18 PM, Dmitriy Shulyak wrote:<br>
><br>
> On Mon, Feb 9, 2015 at 1:35 PM, Przemyslaw Kaminski<br>
</span><div><div class="h5">> <<a href="mailto:pkaminski@mirantis.com">pkaminski@mirantis.com</a> <mailto:<a href="mailto:pkaminski@mirantis.com">pkaminski@mirantis.com</a>>> wrote:<br>
><br>
>> Well i think there should be finished_at field anyway, why not<br>
>> to add it for this purpose?<br>
><br>
> So you're suggesting to add another column and modify all tasks<br>
> for this one feature?<br>
><br>
><br>
> Such things as time stamps should be on all tasks anyway.<br>
><br>
>><br>
>> I dont actually recall what was the reason to delete them, but<br>
>> if it happens imo it is ok to show right now that network<br>
>> verification wasnt performed.<br>
><br>
> Is this how one does predictible and easy to understand software?<br>
> Sometimes we'll say that verification is OK, othertimes that it<br>
> wasn't performed?<br>
><br>
> In my opinion the questions that needs to be answered - what is<br>
> the reason or event to remove verify_networks tasks history?<br>
><br>
>><br>
>> 3. Just having network verification status as ready is NOT<br>
>> enough. From the UI you can fire off network verification for<br>
>> unsaved changes. Some JSON request is made, network configuration<br>
>> validated by tasks and RPC call made returing that all is OK for<br>
>> example. But if you haven't saved your changes then in fact you<br>
>> haven't verified your current configuration, just some other one.<br>
>> So in this case task status 'ready' doesn't mean that current<br>
>> cluster config is valid. What do you propose in this case? Fail<br>
>> the task on purpose?<br>
><br>
> Issue #3 I described is still valid -- what is your solution in<br>
> this case?<br>
><br>
> Ok, sorry. What do you think if in such case we will remove old<br>
> tasks? It seems to me that is correct event in which old<br>
> verify_networks is invalid anyway, and there is no point to store<br>
> history.<br>
<br>
</div></div>Well, not exactly. Configure networks, save settings, do network check<br>
all assume that all went fine. Now change one thing without saving,<br>
check settings, didn't pass but it doesn't affect the flag because<br>
that's some different configuration from the saved one. And your<br>
original cluster is OK still. So in this case user will have to yet<br>
again run the original check. The plus of the network_check_status<br>
column is actually you don't need to store any history -- task can be<br>
deleted or whatever and still last checked saved configuration<br>
matters. User can perform other checks 'for free' and is not required<br>
to rerun the working configuration checks.<br>
<br>
With data depending on tasks you actually have to store a lot of<br>
history because you need to keep last working saved configuration --<br>
otherwise user will have to rerun original configuration. So from<br>
usability point of view this is a worse solution.<br>
<span class=""><br>
><br>
><br>
> As far as I understand, there's one supertask 'verify_networks'<br>
> (called in nailgu/task/manager.py line 751). It spawns other tasks<br>
> that do verification. When all is OK verify_networks calls RPC's<br>
> 'verify_networks_resp' method and returns a 'ready' status and at<br>
> that point I can inject code to also set the DB column in cluster<br>
> saying that network verification was OK for the saved<br>
> configuration. Adding other tasks should in no way affect this<br>
> behavior since they're just subtasks of this task -- or am I<br>
> wrong?<br>
><br>
><br>
> It is not that smooth, but in general yes - it can be done when<br>
> state of verify_networks is changed. But lets say we have<br>
> some_settings_verify task? Would be it valid to add one more field<br>
> on cluster model, like some_settings_status?<br>
<br>
</span>Well, why not? Cluster deployment is a task and it's status is saved<br>
in cluster colum and not fetched from tasks. As you see the logic of<br>
network task verification is not simply based on ready/error status<br>
reading but more subtle. What other settings you have in mind? I guess<br>
when we have more of them one can create a separate table to keep<br>
them, but for now I don't see a point in doing this.<br>
<div class="HOEnZb"><div class="h5"><br>
P.<br>
<br>
><br>
><br>
><br>
><br>
><br>
> __________________________________________________________________________<br>
><br>
><br>
OpenStack Development Mailing List (not for usage questions)<br>
> Unsubscribe:<br>
> <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
><br>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr">Vitaly Kramskikh,<br>Fuel UI Tech Lead,<br>Mirantis, Inc.</div></div></div></div>
</div>