Issue 3949041: Add a connection manager (Closed)
Patch Set 1 #
Total comments: 28
Patch Set 2 : Removed reference to CONFIG.login etc and reviewed the UI of the manager #Patch Set 3 : Fix empty section name #Patch Set 4 : Compatibility with python2.5 #
Total comments: 6
Patch Set 5 : Added asynchronousity to db loading widget #Patch Set 6 : Fix section name in entry and add default button #Patch Set 7 : Fix for set_activates_default #Patch Set 8 : python2.5 compatiblity #Patch Set 9 : Added more checks when fetching database list #Patch Set 10 : Change DB loading thread for windows #Patch Set 11 : Handle database creation, cache db list #Patch Set 12 : Adding post create database support #Patch Set 13 : Added compatibility with python2.5 #Patch Set 14 : Fix for cedric comments #Patch Set 15 : Updated to trunk #Patch Set 16 : Fix profile disparition #
Total comments: 7
Patch Set 17 : Updated to latest trunk #Patch Set 18 : Fix profile disparition and KNOWN_HOST issues #
Total comments: 1
Patch Set 19 : Better handling of focus-out in editable tree #
MessagesTotal messages: 66
Nice Idea.
Traceback (most recent call last):
File "/tryton/gui/window/dblogin.py", line 166, in profile_selected
entry_value = self.profiles.get(profile_name, field)
File "/ConfigParser.py", line 531, in get
raise NoSectionError(section)
NoSectionError: No section: ''
I was able to create a Profile without name.
Sign in to reply to this message.
I got error message "'gtk.Dialog' object has no attribute 'destoy'" when press esc on login dialog.
Sign in to reply to this message.
When removing a profile:
Traceback (most recent call last):
File "/tryton/gui/window/dblogin.py", line 309, in profile_changed
profile = self.profile_store[position][0]
IndexError: could not find tree path
Sign in to reply to this message.
Interesting, thx. Will test ASAP. Found 2 microissues at first glance. http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (right): http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:10: unconventional empty line http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:344: self.dialog.destoy() typo
Sign in to reply to this message.
http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (left): http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#old... tryton/gui/window/dblogin.py:149: CONFIG['login.server'] = host What about those parameters? They still exist but could be not sync with the profile http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (right): http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:28: self.dialog = gtk.Dialog(title=_(u'Profile Editor'), parent=parent, Why using unicode here? http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:130: self.dialog.destroy() You should call self.parent.present() http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:147: model.append((profile_name,)) You must test if the profile is not yet in the model http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:295: self.profile_cfg = os.path.join(get_config_dir(), 'profile.cfg') profiles.cfg as there will be many profiles http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:295: self.profile_cfg = os.path.join(get_config_dir(), 'profile.cfg') We should have a default entry in profile.cfg if the file doesn't exist with connection to: - localhost - demox.y.tryton.org http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:296: self.profiles = ConfigParser.ConfigParser({'username': ''}) Why not setting username by default to 'admin'? http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:302: dia = DBListEditor(self.dialog, self.profile_store, self.profiles) It seems that the combo_profile doesn't set a profile when it was empty before editing. http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:313: self.entry_login.set_editable(False) I will let the login editable http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:337: result = (self.profiles.get(profile, 'username'), Doesn't work if there is no username in the profile
Sign in to reply to this message.
http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (right): http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:10: On 2011/01/10 16:58:29, yangoon wrote: > unconventional empty line I like to separate local imports from system imports. I'll remove the line to follow tryton spec http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:28: self.dialog = gtk.Dialog(title=_(u'Profile Editor'), parent=parent, On 2011/01/10 17:23:53, ced wrote: > Why using unicode here? So that later 2to3 will treat those strings as strings (I saw that ~1 year ago). http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:296: self.profiles = ConfigParser.ConfigParser({'username': ''}) On 2011/01/10 17:23:53, ced wrote: > Why not setting username by default to 'admin'? I think the default should be to ask the user what her username is. http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:313: self.entry_login.set_editable(False) On 2011/01/10 17:23:53, ced wrote: > I will let the login editable I would not this is some kind of security (not strong security but enough for many lambda users)
Sign in to reply to this message.
On 2011/01/10 16:55:05, ced wrote: > When removing a profile: > > Traceback (most recent call last): > File "/tryton/gui/window/dblogin.py", line 309, in profile_changed > profile = self.profile_store[position][0] > IndexError: could not find tree path I can not reproduce it
Sign in to reply to this message.
http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (left): http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#old... tryton/gui/window/dblogin.py:149: CONFIG['login.server'] = host On 2011/01/10 17:23:53, ced wrote: > What about those parameters? > They still exist but could be not sync with the profile Maybe create a migration script to run on the client side ? Or should we handle this in dblogin ? http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (right): http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:295: self.profile_cfg = os.path.join(get_config_dir(), 'profile.cfg') On 2011/01/10 17:23:53, ced wrote: > We should have a default entry in profile.cfg if the file doesn't exist with > connection to: > - localhost > - http://demox.y.tryton.org You can not have two section values as default. But we could create a predefined file if the config file does not exist.
Sign in to reply to this message.
http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (right): http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:296: self.profiles = ConfigParser.ConfigParser({'username': ''}) On 2011/01/10 17:45:48, nicoe wrote: > On 2011/01/10 17:23:53, ced wrote: > > Why not setting username by default to 'admin'? > > I think the default should be to ask the user what her username is. At the first time, the only login exists is 'admin'. http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:313: self.entry_login.set_editable(False) On 2011/01/10 17:45:48, nicoe wrote: > On 2011/01/10 17:23:53, ced wrote: > > I will let the login editable > > I would not this is some kind of security (not strong security but enough for > many lambda users) This is absolutely not security. It will just make annoying to switch from one user to an other (like re-login as admin for admin task).
Sign in to reply to this message.
On 2011/01/10 17:56:19, nicoe wrote: > On 2011/01/10 16:55:05, ced wrote: > > When removing a profile: > > > > Traceback (most recent call last): > > File "/tryton/gui/window/dblogin.py", line 309, in profile_changed > > profile = self.profile_store[position][0] > > IndexError: could not find tree path > > I can not reproduce it It is when you try to delete a second duplicate profile
Sign in to reply to this message.
http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (right): http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:296: self.profiles = ConfigParser.ConfigParser({'username': ''}) On 2011/01/10 18:15:39, ced wrote: > On 2011/01/10 17:45:48, nicoe wrote: > > On 2011/01/10 17:23:53, ced wrote: > > > Why not setting username by default to 'admin'? > > > > I think the default should be to ask the user what her username is. > > At the first time, the only login exists is 'admin'. Because the only login that existed a while ago was 'admin' we should always default to that one ? I don't think so. http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:313: self.entry_login.set_editable(False) On 2011/01/10 18:15:39, ced wrote: > On 2011/01/10 17:45:48, nicoe wrote: > > On 2011/01/10 17:23:53, ced wrote: > > > I will let the login editable > > > > I would not this is some kind of security (not strong security but enough for > > many lambda users) > > This is absolutely not security. It will just make annoying to switch from one > user to an other (like re-login as admin for admin task). It is not real security but prevent user changing the username (which is what I expect since the profile defines a username).
Sign in to reply to this message.
http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (left): http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#old... tryton/gui/window/dblogin.py:149: CONFIG['login.server'] = host On 2011/01/10 17:59:57, nicoe wrote: > On 2011/01/10 17:23:53, ced wrote: > > What about those parameters? > > They still exist but could be not sync with the profile > > Maybe create a migration script to run on the client side ? > Or should we handle this in dblogin ? They are still use in the database management. I think they should be dropped and replace by the profile all over the place (and in some place just ignoring the database name of the profile). http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (right): http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:295: self.profile_cfg = os.path.join(get_config_dir(), 'profile.cfg') On 2011/01/10 17:59:57, nicoe wrote: > On 2011/01/10 17:23:53, ced wrote: > > We should have a default entry in profile.cfg if the file doesn't exist with > > connection to: > > - localhost > > - http://demox.y.tryton.org > > You can not have two section values as default. But we could create a predefined > file if the config file does not exist. In fact it is not possible to get a localhost default connection, but it will be great for demo.
Sign in to reply to this message.
http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (right): http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:200: elif dbs == 0: We lost the shortcut to create a new database.
Sign in to reply to this message.
http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (right): http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:296: self.profiles = ConfigParser.ConfigParser({'username': ''}) On 2011/01/10 18:22:40, nicoe wrote: > On 2011/01/10 18:15:39, ced wrote: > > On 2011/01/10 17:45:48, nicoe wrote: > > > On 2011/01/10 17:23:53, ced wrote: > > > > Why not setting username by default to 'admin'? > > > > > > I think the default should be to ask the user what her username is. > > > > At the first time, the only login exists is 'admin'. > > Because the only login that existed a while ago was 'admin' we should always > default to that one ? I don't think so. No because the first time a user try Tryton, he don't know the login to use. http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:313: self.entry_login.set_editable(False) On 2011/01/10 18:22:40, nicoe wrote: > On 2011/01/10 18:15:39, ced wrote: > > On 2011/01/10 17:45:48, nicoe wrote: > > > On 2011/01/10 17:23:53, ced wrote: > > > > I will let the login editable > > > > > > I would not this is some kind of security (not strong security but enough > for > > > many lambda users) > > > > This is absolutely not security. It will just make annoying to switch from one > > user to an other (like re-login as admin for admin task). > > It is not real security but prevent user changing the username (which is what I > expect since the profile defines a username). login name should a default value, not a constraint. There is no advantage to have a constraint there.
Sign in to reply to this message.
when using "Default" as profile name I get this error:
ERROR:common.message:Traceback (most recent call last):
File "/tryton/gui/window/dblogin.py", line 138, in profile_create
self.profiles.add_section(profile_name)
File "/ConfigParser.py", line 242, in add_section
raise ValueError, 'Invalid section name: %s' % section
ValueError: Invalid section name: Default
Sign in to reply to this message.
http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (right): http://codereview.appspot.com/3949041/diff/1/tryton/gui/window/dblogin.py#new... tryton/gui/window/dblogin.py:296: self.profiles = ConfigParser.ConfigParser({'username': ''}) On 2011/01/10 18:30:37, ced wrote: > On 2011/01/10 18:22:40, nicoe wrote: > > On 2011/01/10 18:15:39, ced wrote: > > > On 2011/01/10 17:45:48, nicoe wrote: > > > > On 2011/01/10 17:23:53, ced wrote: > > > > > Why not setting username by default to 'admin'? > > > > > > > > I think the default should be to ask the user what her username is. > > > > > > At the first time, the only login exists is 'admin'. > > > > Because the only login that existed a while ago was 'admin' we should always > > default to that one ? I don't think so. > > No because the first time a user try Tryton, he don't know the login to use. For the first time agreed. But on subsequent logins it should be empty or eventually set to the last login (if someone wishes this behaviour, this should be switchable). It is bad behaviour (Redmond-like) to push users to login constantly as admin.
Sign in to reply to this message.
On 2011/01/10 22:50:58, yangoon wrote: > For the first time agreed. But on subsequent logins it should be empty or > eventually set to the last login (if someone wishes this behaviour, this should > be switchable). It is bad behaviour (Redmond-like) to push users to login > constantly as admin. My vote, too.
Sign in to reply to this message.
On 2011/01/11 15:04:29, udono wrote: > On 2011/01/10 22:50:58, yangoon wrote: > > For the first time agreed. But on subsequent logins it should be empty or > > eventually set to the last login (if someone wishes this behaviour, this > should > > be switchable). It is bad behaviour (Redmond-like) to push users to login > > constantly as admin. > My vote, too. Ok we should just set it in the login windows just after database creation.
Sign in to reply to this message.
When adding a new profile, with empty options (just click on add, enter name,
press ok) the following error raises:
Traceback (most recent call last):
File "/tryton/gui/window/dblogin.py", line 382, in profile_changed
username = self.profiles.get(profile, 'username')
File "/ConfigParser.py", line 531, in get
raise NoSectionError(section)
NoSectionError: No section: ''
Sign in to reply to this message.
After last reported error raise:
ERROR:common.message:No section: ''
ERROR:common.message:Traceback (most recent call last):
File "/tryton/gui/window/dblogin.py", line 382, in profile_changed
username = self.profiles.get(profile, 'username')
File "/ConfigParser.py", line 531, in get
raise NoSectionError(section)
NoSectionError: No section: ''
Sign in to reply to this message.
On 2011/01/13 14:43:49, nicoe wrote: > Fix empty section name Now it works better, but when I do: * Press Add * Enter Name: 12345 * Put cursor into field Hostname with 3x pressing tab * Press Ok The a new connection with name 12345 is created with the preferences of another connection.
Sign in to reply to this message.
On 2011/01/13 14:57:03, udono wrote: > On 2011/01/13 14:43:49, nicoe wrote: > > Fix empty section name > > Now it works better, but when I do: > > * Press Add > * Enter Name: 12345 > * Put cursor into field Hostname with 3x pressing tab > * Press Ok > > The a new connection with name 12345 is created with the preferences of another > connection. I did not suceed in repeating this error.
Sign in to reply to this message.
On 2011/01/13 15:44:57, nicoe wrote: > On 2011/01/13 14:57:03, udono wrote: > > On 2011/01/13 14:43:49, nicoe wrote: > > > Fix empty section name > > Now it works better, but when I do: > > * Press Add > > * Enter Name: 12345 > > * Put cursor into field Hostname with 3x pressing tab Here you need to use tab key to move from name entry to field Hostname!
Sign in to reply to this message.
On 2011/01/13 16:29:17, udono wrote: > On 2011/01/13 15:44:57, nicoe wrote: > > On 2011/01/13 14:57:03, udono wrote: > > > On 2011/01/13 14:43:49, nicoe wrote: > > > > Fix empty section name > > > Now it works better, but when I do: > > > * Press Add > > > * Enter Name: 12345 > > > * Put cursor into field Hostname with 3x pressing tab > Here you need to use tab key to move from name entry to field Hostname! Even when doing that I can not reproduce the bug Here is what I am doing: Alt-A 12345 Tab Tab Tab Tab Tab Tab Tab (I am now on the Ok button) Enter Then I read the config file created and the section 12345 is empty
Sign in to reply to this message.
Here is my initial feedback; 1) Patch applied and operates without change on Mac OS 10.6.6. 2) No default Profile is selected upon initial presentation of the Login Window (Note that I did remove ~/.config/tryton before execution.) 3) Simply clicking the empty Profile drop down populates the drop down with the demo server. 4) The Manage profiles window seems to be synchronous (that is, it blocks while the server is contacted to enumerate the database list.) This is a problem if the user is of even moderate distance from the server. For example, my latency to the demo server is 115ms rtt from me, and this caused the window to block for ~ 2 seconds. 5) when adding a new entry to the profile list, the width of the list widget may change. (for instance, try naming the profile 'localhost', then tab to the next control) The same is true when the user provides a port number... 6) When adding a new server, the connection manager should not try to enumerate the databases until a hostname and port been provided. 7) The connect manager does not seem to assume a default port. It should default to 8070. Otherwise, this is a great concept! --phil
Sign in to reply to this message.
As we don't use anymore the login section of CONFIG, it should be dropped. Also I think it will be good to remember last profile use and set it as default value and add a command line options to pass a profile name. http://codereview.appspot.com/3949041/diff/32001/tryton/common/common.py File tryton/common/common.py (right): http://codereview.appspot.com/3949041/diff/32001/tryton/common/common.py#newc... tryton/common/common.py:64: dbtoload = '' Set default value in method signature http://codereview.appspot.com/3949041/diff/32001/tryton/gui/window/dbcreate.py File tryton/gui/window/dbcreate.py (left): http://codereview.appspot.com/3949041/diff/32001/tryton/gui/window/dbcreate.p... tryton/gui/window/dbcreate.py:406: if self.sig_login: Why did you remove sig_login callback? If it is no more required just drop it. http://codereview.appspot.com/3949041/diff/32001/tryton/gui/window/dbcreate.py File tryton/gui/window/dbcreate.py (right): http://codereview.appspot.com/3949041/diff/32001/tryton/gui/window/dbcreate.p... tryton/gui/window/dbcreate.py:306: admin_passwd = self.entry_adminpasswd Why moving this line which did not change? http://codereview.appspot.com/3949041/diff/32001/tryton/gui/window/dbcreate.p... tryton/gui/window/dbcreate.py:308: change_button.connect_after('clicked', self.server_change, self.dialog) Why moving this line which did not change? http://codereview.appspot.com/3949041/diff/32001/tryton/gui/window/dbdumpdrop.py File tryton/gui/window/dbdumpdrop.py (right): http://codereview.appspot.com/3949041/diff/32001/tryton/gui/window/dbdumpdrop... tryton/gui/window/dbdumpdrop.py:45: pass Then no need to test with "if".
Sign in to reply to this message.
http://codereview.appspot.com/3949041/diff/32001/tryton/common/common.py File tryton/common/common.py (right): http://codereview.appspot.com/3949041/diff/32001/tryton/common/common.py#newc... tryton/common/common.py:64: dbtoload = '' On 2011/01/14 11:38:04, ced wrote: > Set default value in method signature No need for this anymore indeed.
Sign in to reply to this message.
Tested with tryton gtk client trunk on Mac OS X 10.6.6. Observations: The client does not create profiles.conf until the profile editor has been activated at least once. It should create it at startup, if absent. Upon initial launch of Tryton, the "Profile" selection is empty. It should be initialized to the demo profile upon initial launch. Profile Editor: The manage profile dialog does not populate the Hostname, port, database, or username fields. I do see default values for the demo in profiles.conf. When adding a new profile, the tab order should go from the profile name to hostname, ... It is not possible to select a profile with the mouse in the Profile Editor. The profile editor will not allow you to create a profile with an empty name... but if the profile is first created with a name, then edited to an empty name, the editor permits this and saves the null profile to profiles.cfg (which throws errors) As the database selection is not populated, I am unable to test any further.
Sign in to reply to this message.
> The client does not create profiles.conf until the profile editor has been > activated at least once. It should create it at startup, if absent. In fact if there is no profiles.conf the client will create one for you. But if you do not opene the profile editor than there is no need to save the default value. > Upon initial launch of Tryton, the "Profile" selection is empty. It should be > initialized to the demo profile upon initial launch. > > Profile Editor: > > The manage profile dialog does not populate the Hostname, port, database, or > username fields. I do see default values for the demo in profiles.conf. I don't understand. You're clicking on 'demo1.9.tryton.org' and nothing is shown in the username/host/port entries ? > When adding a new profile, the tab order should go from the profile name to > hostname, ... In fact typing return works that way. Tab go to the Add button. > It is not possible to select a profile with the mouse in the Profile Editor. It should. Are you trying to access the demo server and then clicking on another profile ? If so then this is the intended behavior : as long as we don't have the response of the distant host we prevent selecting another profile. > The profile editor will not allow you to create a profile with an empty name... > but if the profile is first created with a name, then edited to an empty name, > the editor permits this and saves the null profile to profiles.cfg (which throws > errors) I have to fix this. > As the database selection is not populated, I am unable to test any further. Did you try with a server running on your host ?
Sign in to reply to this message.
Inline... On Jan 21, 2011, at 6:23 AM, nicolas.evrard@b2ck.com wrote: > In fact if there is no profiles.conf the client will create one for you. > But if you do not opene the profile editor than there is no need to save > the default value. Ok, I suppose that is true. But, if there is no profile, the "demo" profile should be selected by default in the Connection window. Eureka.... I found the problem. gtk.gdk.threads_init is disabled on the Mac in tryton/client.py. This was done to solve issues during the 1.8.0 release, but it seems that it is working better in later releases (and must also be enabled for the URL patch). It still does cause the application to block in some instances, which I'm trying to assess. Anyways, after re-enabling this, it works mostly fine. Still the issue with the possibility to make a profile name blank, though. Otherwise, looks great. Sorry for not investigating further at first.... --phil
Sign in to reply to this message.
On 2011/01/21 15:11:10, pheller_me.com wrote: > > In fact if there is no profiles.conf the client will create one for you. > > But if you do not opene the profile editor than there is no need to save > > the default value. > > Ok, I suppose that is true. But, if there is no profile, the "demo" profile > should be selected by default in the Connection window. It is already the case on my box. I have to push this change on codereview. > Eureka.... I found the problem. gtk.gdk.threads_init is disabled on the Mac in > tryton/client.py. This was done to solve issues during the 1.8.0 release, but > it seems that it is working better in later releases (and must also be enabled > for the URL patch). It still does cause the application to block in some > instances, which I'm trying to assess. We should build a client for windows to be sure that this thread issue does not happend under this OS too ... > Sorry for not investigating further at first.... No problem.
Sign in to reply to this message.
Neso must be updated to work with the new windows
Sign in to reply to this message.
doc/usage.rst should be updated
Sign in to reply to this message.
Usability is getting better and better. Good job nicoe! I collected some minor issues for patch set 8: 1. When I open the manage profile, the first profile is auto selected on the left. But the values on the right are not updated. I need to chose another Profile and back again to the first, to see the values. 2. Additionally I propose a change of the focus order: * creating a new profile left, * Tab * Focus in field Hostname 3. Adding a profile with the same name as a profile which already exist is possible. The new profile name is empty. But I can encode all preferences on the right. * When I close the login, * and re-login (ctrl-O), * open connection manager, * click to the profile with empty name, * press enter, => the profile and my preferences disappears and be away. My suggestion for Profile Manager: * Add a cancel button * Not accept identical profile names * Set the entry on the right side and the ok-button read-only as long as there is no valid profile chosen. 4. Give the Manage Profiles Button an accelerator key 5. Additionally it would be nice when it is possible to rearrange the list of profiles.
Sign in to reply to this message.
On 2011/02/09 14:45:54, udono wrote: > 1. When I open the manage profile, the first profile is auto selected on the > left. But the values on the right are not updated. Fixed. Thx. > 3. Adding a profile with the same name as a profile which already exist is > possible. Fixed. Thx. > 4. Give the Manage Profiles Button an accelerator key Fixed. Thx.
Sign in to reply to this message.
On 2011/02/09 14:45:54, udono wrote: > * Add a cancel button Fixed. Thx.
Sign in to reply to this message.
Incomplete profile should not be display in login window. The first database of the server should be selected by default. Switching focus from Port/Hostname should not change the database selected if parameter did not change. Escape key doesn't close the login windows any more. The value 8070 should be as a default port. When clicking on create button when there is no connection, it should not be allow to change the server connection. When clicking on any database action from the menu, the server connection field is not filled. When there is an error at login, the login window should come back. The fetch of database list doesn't work on IPv6. The selection list of database backup and database drop is too small. The restore button is not activated after setting missing server connection. After a restore the new database list is not fetched in the connection manager.
Sign in to reply to this message.
On 2011/02/15 15:56:27, ced wrote: > Incomplete profile should not be display in login window. Done. > The first database of the server should be selected by default. Done. > Switching focus from Port/Hostname should not change the database selected if > parameter did not change. Done. > Escape key doesn't close the login windows any more. Done. > The value 8070 should be as a default port. Done. > When clicking on any database action from the menu, the server connection field > is not filled. It is on purpose. I don't think we should use the information from the default profile. > When there is an error at login, the login window should come back. I tested with a wrong password and it works. In which example did not it work ? > The fetch of database list doesn't work on IPv6. Really strange since I do not toouch the connection code. > The restore button is not activated after setting missing server connection. I don't understand. TODO in the plane: > After a restore the new database list is not fetched in the connection manager. > When clicking on create button when there is no connection, it should not be > allow to change the server connection. > The selection list of database backup and database drop is too small.
Sign in to reply to this message.
> TODO in the plane: > > After a restore the new database list is not fetched in the connection > > manager. Done > > When clicking on create button when there is no connection, it should not be > > allow to change the server connection. Done > > The selection list of database backup and database drop is too small.
Sign in to reply to this message.
Still missing Neso patch
Sign in to reply to this message.
Content of profile_tree is cleared on focus out http://codereview.appspot.com/3949041/diff/97001/tryton/common/common.py File tryton/common/common.py (right): http://codereview.appspot.com/3949041/diff/97001/tryton/common/common.py#newc... tryton/common/common.py:1025: KNOWN_DBS = {} Should not be a class attribute because if there is connection issue (missing network) the first time you try to fetch DB then it is never refreshed. http://codereview.appspot.com/3949041/diff/97001/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (right): http://codereview.appspot.com/3949041/diff/97001/tryton/gui/window/dblogin.py... tryton/gui/window/dblogin.py:312: def refreshdblist(self, host, port, dbname=None): Not used (never called)
Sign in to reply to this message.
http://codereview.appspot.com/3949041/diff/97001/tryton/common/common.py File tryton/common/common.py (right): http://codereview.appspot.com/3949041/diff/97001/tryton/common/common.py#newc... tryton/common/common.py:1025: KNOWN_DBS = {} On 2011/03/10 22:15:02, ced wrote: > Should not be a class attribute because if there is connection issue (missing > network) the first time you try to fetch DB then it is never refreshed. Done. http://codereview.appspot.com/3949041/diff/97001/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (right): http://codereview.appspot.com/3949041/diff/97001/tryton/gui/window/dblogin.py... tryton/gui/window/dblogin.py:312: def refreshdblist(self, host, port, dbname=None): On 2011/03/10 22:15:02, ced wrote: > Not used (never called) Done.
Sign in to reply to this message.
http://codereview.appspot.com/3949041/diff/97001/tryton/common/common.py File tryton/common/common.py (right): http://codereview.appspot.com/3949041/diff/97001/tryton/common/common.py#newc... tryton/common/common.py:1025: KNOWN_DBS = {} On 2011/03/11 11:11:31, nicoe wrote: > On 2011/03/10 22:15:02, ced wrote: > > Should not be a class attribute because if there is connection issue (missing > > network) the first time you try to fetch DB then it is never refreshed. > > Done. Don't see it. http://codereview.appspot.com/3949041/diff/97001/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (right): http://codereview.appspot.com/3949041/diff/97001/tryton/gui/window/dblogin.py... tryton/gui/window/dblogin.py:312: def refreshdblist(self, host, port, dbname=None): On 2011/03/11 11:11:31, nicoe wrote: > On 2011/03/10 22:15:02, ced wrote: > > Not used (never called) > > Done. Don't see it.
Sign in to reply to this message.
On 2011/03/11 11:28:08, ced wrote: > http://codereview.appspot.com/3949041/diff/97001/tryton/common/common.py > File tryton/common/common.py (right): > > http://codereview.appspot.com/3949041/diff/97001/tryton/common/common.py#newc... > tryton/common/common.py:1025: KNOWN_DBS = {} > On 2011/03/11 11:11:31, nicoe wrote: > > On 2011/03/10 22:15:02, ced wrote: > > > Should not be a class attribute because if there is connection issue > (missing > > > network) the first time you try to fetch DB then it is never refreshed. > > > > Done. > > Don't see it. Basic time concepts are learned in first/second grade.
Sign in to reply to this message.
http://codereview.appspot.com/3949041/diff/97001/tryton/common/common.py File tryton/common/common.py (right): http://codereview.appspot.com/3949041/diff/97001/tryton/common/common.py#newc... tryton/common/common.py:1025: KNOWN_DBS = {} On 2011/03/11 11:28:08, ced wrote: > On 2011/03/11 11:11:31, nicoe wrote: > > On 2011/03/10 22:15:02, ced wrote: > > > Should not be a class attribute because if there is connection issue > (missing > > > network) the first time you try to fetch DB then it is never refreshed. > > > > Done. > > Don't see it. No need to say done if nobody can see it.
Sign in to reply to this message.
http://codereview.appspot.com/3949041/diff/105007/tryton/gui/window/dblogin.py File tryton/gui/window/dblogin.py (right): http://codereview.appspot.com/3949041/diff/105007/tryton/gui/window/dblogin.p... tryton/gui/window/dblogin.py:236: renderer.has_text = (path, editable.get_text()) Why don't you put the get_text() into the model?
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
