[virt-tools-list] [PATCH] Drop user back to 'open conn' dialog if connecting fails
crobinso at redhat.com
Wed Aug 7 22:50:32 UTC 2013
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
>> 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.
More information about the virt-tools-list