This allows you to raise a warning if dependencies are not unmet

[Danisan]

Could see that raise fully stops the system and to error 500

[Yenthe666]

@Danisan thanks for the contribution, this looks neat! Did you write this because you saw this? https://github.com/Yenthe666/auto_backup/issues/38 I'm going to ask grexe to test this and give his feedback too, if it works fine from the terminal too I will merge it in.

[grexe]

I'd prefer the error to be logged instead of silently pass'ed, though. From the code I can see it should not cause any issues...

[grexe]

hmmm still have no luck installing this module on 9.0 nightly though, while other modules worked. Can't find any difference, folder and permissions are ok, using the default addons path /usr/lib/python2.7/dist-packages/openerp/addons/

[Danisan]

@grexe, with this you will have a more friendly warning about the lack of dependencies. @Yenthe666 actually I wasn't following the issues here, just a customer of mine installed the module and got fightened when everything was broken for him.

[gaikaz]

This is a very good forgotten PR that is still relevant even in later versions of Odoo. I only have one suggestion (something that @grexe already suggested in his issue #38). Instead of passing, _logger can be initialized above the try/except block and used to log the same error from currently raised ImportError. This would give more information in the logs for the sys. admin like the raise, yet won't stop further code execution like the pass. I already tested this approach on Odoo v10.

[Yenthe666]

@gaikaz thanks for the feedback. I don't feel like doing pass in an Exception makes sense though. It feels like silently covering up a serious problem. Without the dependency the module wouldn't work for remote backups anyways so should we really allow a silent pass?

[gaikaz]

@Yenthe666 , sorry, I wasn't very clear. I fully agree and this is exactly what I suggest changing to make this PR perfect. :) Replace

raise ImportError('This module needs pysftp to automaticly write backups to the FTP through SFTP. Please install pysftp on your system. (sudo pip install pysftp)')

with

_logger.error('This module needs pysftp to automaticly write backups to the FTP through SFTP. Please install pysftp on your system. (sudo pip install pysftp)')

instead of pass So it logs the error but doesn't stop the code like raise does.

[Yenthe666]

Yeah that is absolutely better but what if you're not looking at logs though? You wouldn't notice that you're missing a dependency. As the scheduler itself also runs in the background you might end up with a scheduler that keeps failing and you thinking that the backup module does not work though? While you're just missing a dependency.

I'm not sure if this warns the user enough. On the other hand, if you only backup locally (why though?) you would not need the extra dependency. In this case this example would then be better again.

[gaikaz]

In this PR @Danisan also adds external_dependencies parameter in the manifest file in commit 5b518fcce4209918744ecc4b38b7c2b6595b9fcb . It prevents installing the module if the dependency is missing (gives a very nice prompt). So you won't even reach the point, where you have a scheduler inserted.

[Yenthe666]

Aha I see! I'll test it soon, thanks. 👍