[virt-tools-list] [PATCH] Drop user back to 'open conn' dialog if connecting fails

[Cole Robinson]

On 08/07/2013 05:49 PM, Giuseppe Scrivano wrote:
> Cole Robinson <crobinso at redhat.com> writes:
> 
>> Please use [PATCH v2] for version 2 of a patch, etc. git format-patch
>> --subject-prefix "PATCH v2"
> 
> thanks, I'll do it.
> 
>> This seems overkill. Why can't we add an argument to show() which skips the
>> reset_state step?
> 
>>
>> Hmm, I guess someone could reopen the connect wizard while the first
>> connection is opening, which would mean the state is wrong when we re-show the
>> wizard.
>>
>> Thinking some more, what we really want here is:
>>
>> Launch 'Open Connection', set parameters, hit 'connect'
>> 'Open Connection' window is desensitized, async progress meter dialog opens up
>> while connection is connecting
>> If connection fails, we drop back to the still open connection wizard with
>> same state still listed.
>>
>> That's how the create/clone/delete wizards work, basically make the operation
>> synchronous. However truth be told I don't care enough about this bug to
>> mandate that we do that. If you want to look into it it might be simple, or it
>> might be a pain.
>>
>> But the above solution is definitely not ideal since it requires duplicating
>> update of UI elements in two places (set_state and reset_state) with the
>> latter in a rarely tested code path.
> 
> I have firstly implemented it because I wanted to take care of that
> case but I agree it is a very uncommon case.  Then I realized that
> set_state can also be useful if we intent to support "Modify connection"
> in future (not really sure it is useful).
> 

Ah, fair point, but if we ever did that, it would be integrated into the 'host
details' connection page, so likely wouldn't use connect.py

> Would you accept the patch if I combine reset_state in set_state, so
> that set_state will accept a set_to_default argument?  If you still
> think that the cost of maintaining the new code is bigger than the
> advantage of fixing that particular case, I can drop set_state.

Given my comment above, I don't think we should have connect.py pulling
details out of a vmmConnection since it doesn't have much of a future besides
facilitating this one corner case. So I'd prefer to drop it.

Thanks,
Cole