On Tuesday 11 April 2017 14:31:06 you wrote: > Sorry, lots of comments. > > Diff comments: > > === modified file 'bin/updater.py' > > --- bin/updater.py 2017-01-17 13:07:11 +0000 > > +++ bin/updater.py 2017-04-11 07:12:45 +0000 > > @@ -445,42 +445,91 @@ > > > > def reconnect_sync_server(): > > """Reconnect the connection manager to the SYNC_SERVER if password > > file > > exists > > > > + If password file is not found but sync_user_login and > > sync_user_password are + found in the config file, set them into the > > connection manager, this way + there is no need to enter them > > manually. > > > > """ > > import tools > > > > + import pooler > > > > credential_filepath = os.path.join(tools.config['root_path'], > > 'unifield-socket.py')> > > - if os.path.isfile(credential_filepath): > > - import base64 > > - import pooler > > - f = open(credential_filepath, 'r') > > - lines = f.readlines() > > - f.close() > > - if lines: > > - try: > > + sync_user_login = tools.config.get('sync_user_login') > > if sync_user_login == 'admin': throw an error. We only want them to be using > this feature with the per-instance user/pass. Throwing an error is may be a bit violent. May be it is better to ignore it an log an error message than preventing the instance from starting ? > > + sync_user_password = tools.config.get('sync_user_password') > > + dbname_list = [] > > + password_from_file = None > > + cr = None > > + > > + try: > > + if os.path.isfile(credential_filepath): > > + import base64 > > + with open(credential_filepath, 'r') as f: > > + lines = f.readlines() > > > > + if lines: > > dbname = base64.decodestring(lines[0]) > > > > - password = base64.decodestring(lines[1]) > > - logger.info('dbname = %s' % dbname) > > - db, pool = pooler.get_db_and_pool(dbname) > > - db, pool = pooler.restart_pool(dbname) # do not remove > > this line, it is required to restart pool not to have - # > > strange behaviour with the connection on web interface - > > - # do not execute this code on server side > > - if not pool.get("sync.server.entity"): > > - cr = db.cursor() > > - # delete the credential file > > - os.remove(credential_filepath) > > - # reconnect to SYNC_SERVER > > - connection_module = > > pool.get("sync.client.sync_server_connection") - > > connection_module.connect(cr, 1, password=password) - > > - # in caes of automatic patching, relaunch the sync > > - # (as the sync that launch the silent upgrade was > > aborted to do the upgrade first) - if > > connection_module.is_automatic_patching_allowed(cr, 1): - > > pool.get('sync.client.entity').sync_withbackup(cr, 1) - > > cr.close() > > - except Exception as e: > > - message = "Impossible to automatically re-connect to the > > SYNC_SERVER using credentials file : %s" - > > logger.error(message % (unicode(e))) > > - > > + dbname_list.append(dbname) > > + password_from_file = base64.decodestring(lines[1]) > > + > > + # in case there is no file for automatic reconnecion after > > patching, + # then get the list of all db and try to set the > > login/password on all + if not dbname_list and sync_user_login and > > sync_user_password: + from service.web_services import db > > + # try to set up the login/password on all databases listed > > + dbname_list = db().exp_list() > > + > > + # remove database name that looks to be SYNC_SERVER > > This is already handled below with "if pool.get("sync.server.entity")" It is not the same : "if pool.get("sync.server.entity")" requires to know the pool which is long and costly operation, so this check permit to remove most of the SYNC_SERVER with a ridiculous cost. > > > + dbname_list = [x for x in dbname_list if 'SYNC' not in x] > > + > > + for dbname in dbname_list: > > + logger.info('reconnect_sync_server: dbname = %s' % dbname) > > + db, pool = pooler.get_db_and_pool(dbname) > > + db, pool = pooler.restart_pool(dbname) # do not remove this > > line, + # it is required to restart pool not to have strange > > behaviour + # with the connection on web interface > > + > > + # do not execute this code on server side > > seems maybe like this should be the first thing in the loop? I don't understand where do you want to put it ? you mean before the line "logger.info('reconnect_sync_server: dbname = %s' % dbname)" ? Then how do you know the pool if you call pool.get("sync.server.entity") before the definition of pool ? > > > + if pool.get("sync.server.entity"): > > + continue This is a second security as we can imagine a sync server instance whitout 'SYNC' in its name. > > + > > + cr = db.cursor() > > + # delete the credential file > > This belongs up above, right after the file is read, not in this loop. > > > + if os.path.isfile(credential_filepath): > > + os.remove(credential_filepath) > > + > > + connection_module = > > pool.get("sync.client.sync_server_connection") + if not > > connection_module: > > + logger.error("Module 'sync.client.sync_server_connection' > > " + "not found on %s, not possible to setup sync > > credentials" + " automatically" % dbname) > > + continue > > + > > + if password_from_file: > > + logger.info('Automatic reconnection to the SYNC_SERVER') > > + # reconnect to SYNC_SERVER > > + connection_module.connect(cr, 1, > > password=password_from_file) + > > + # in caes of automatic patching, relaunch the sync > > caes -> cases > Also: It seems like this will result on a sync on every server start when > sync_user_password is in the config file. (Because production instances run > with automatic patching enabled all the time.) why do you think that ? If you read the code carefully, you will see there is two options: 1- sync_user_login and sync_user_password are defined in the config file 2- automatic patching is activated and password_from_file is not None (mean we are during a restart during patch apply) if the option 2, then the option 1 is ignored as the option 2 already set up the credential AND connect to the SYNC_SERVER otherwise, if not option 2 but option 1, then credential are set BUT as requested in the ticket, the instance is NOT connected to SYNC_SERVER and NO sync is launched. If you understand something else, maybe I have to improve the comments and/or code clarity. > > + # (as the sync that launch the silent upgrade was aborted > > to do the upgrade first) + if > > connection_module.is_automatic_patching_allowed(cr, 1): + > > pool.get('sync.client.entity').sync_withbackup(cr, 1) + > > break > > + else: > > + connection_ids = connection_module.search(cr, 1, []) > > + data_to_write = { > > + 'login': sync_user_login, > > + 'password': sync_user_password, > > + } > > + if connection_ids: > > + logger.info('Automatic set up of sync connection > > credentials') + connection_module.write(cr, 1, > > connection_ids, data_to_write) + cr.commit() > > + except Exception as e: > > + if os.path.isfile(credential_filepath) or password_from_file: > > + message = "Impossible to automatically re-connect to the > > SYNC_SERVER using credentials file: %s" + else: > > + message = "Error durring the automatic setting of > > synchronization credentials on the sync_server_connection: %s" + > > logger.error(message % (unicode(e))) > > + finally: > > + if cr is not None: > > + cr.close() > > > > def check_mako_xml(): > > """ > > > > === modified file 'doc/openerp-server.conf' > > --- doc/openerp-server.conf 2017-03-01 14:42:57 +0000 > > +++ doc/openerp-server.conf 2017-04-11 07:12:45 +0000 > > @@ -50,3 +50,9 @@ > > > > # for Weeks (0=Monday). See > > # > > https://docs.python.org/2/library/logging.handlers.html#timedrotatingfil > > ehandler log_user_xmlrpc_when = D > > > > + > > +# credentials to connect to the SYNC_SERVER > > +# if define, it will automatically setup this credentials after server > > start > Credentials to connect to the sync server. If both are not False, the server > will automatically enter synchronization state "Connected" on startup. ONLY > FOR USE WITH per-instance sync username/password. I think there is some misunderstanding of the ticket from me or from you here: "So here we have to reuse updater.py reconnect_sync_server, but if the user/password are retrieved in the config file we do NOT reconnect the instance but only set the password." From that ticket comment, I understand that this ticket will NOT autoconnect the instance to the SYNC_SERVER but just set up the credentials, so it means state "Not Connected". Am I wrong ? > > > +# then the user just have to mannually connect > > +sync_user_login = False > > +sync_user_password = False -- Fabien MORIN TeMPO Consulting 20, avenue de la Paix 67000 Strasbourg France http://www.tempo-consulting.fr Tel : +33 3 88 56 82 16 Fax : +33 9 70 63 35 46