Review: Disapprove Hello,
The post_load hook for starting the server does not need to be fixed in addons, it just needed to be reactivated dynamically on the server-side (and that's done now - after server revision 4031). Invoking the start_server() method directly is incorrect because it will not be done at the right time, and defeats the purpose of the post_load hook. This is likely the reason why you had an issue with database creation from command-line. The creation of the File and Folder that are triggering error messages are fine, you don't need to remove them. The test is checking that the creation is denied, and it works as expected. It would be nice to hide the error message from the FTP service but that does not matter much, let's not waste time on this. Finally, as far as I can see all the cases where you added cr.rollback() calls are unnecessary: - no point at the end of the document_ftp_test2.yml file, the tests are rolled back automatically - no point after the assert lines document_ftp_test4.yml, the assert did not alter the database state, and they always follow a cr.commit() that has already reset the transaction state. I've double-checked at server r.4061 with addons r.6587 and the document_ftp module now seems to install and start fine, so we can close this bug - no further changes seem to be needed. Thanks for your efforts -- https://code.launchpad.net/~openerp-dev/openobject-addons/trunk-bug-document_ftp-sbh/+merge/86517 Your team OpenERP R&D Team is subscribed to branch lp:~openerp-dev/openobject-addons/trunk-bug-document_ftp-sbh. _______________________________________________ Mailing list: https://launchpad.net/~openerp-dev-gtk Post to : [email protected] Unsubscribe : https://launchpad.net/~openerp-dev-gtk More help : https://help.launchpad.net/ListHelp
