On 08/07/2013 05:49 PM, Giuseppe Scrivano wrote: > Cole Robinson <crobinso@xxxxxxxxxx> 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 _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
