New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python hooks #535

Open
wants to merge 2 commits into
base: stretch-unstable
from

Conversation

Projects
None yet
2 participants
@alexAubin
Copy link
Member

alexAubin commented Sep 1, 2018

The problem

Python hooks are required for the new diagnosis system modularity / extendability, and more generally if we want to implement some hooks in python for some reason. This could be the case for example of the famous app ssowatconf command which should be a classical regen-conf thing but can't be so far because it has too many python calls and json stuff to be implemented in bash.

Solution

Split the behavior of the hook_exec function depending on the nature of the hook. Python hooks are to be modules loaded by yunohost and expected to have a main function.

PR Status

Almost done but might still move depending on #534

How to test

...

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 :
  • Deep review 0/1 :

@alexAubin alexAubin referenced this pull request Sep 1, 2018

Open

[wip] New diagnosis system #534

0 of 4 tasks complete
@zamentur

This comment has been minimized.

Copy link
Contributor

zamentur commented Nov 30, 2018

Ok for diagnosys example.

Note: isn't a risk to include some python code that depends of code from a previous version of YunoHost. I have the same sissue with the module mechanism used by menu_ynh.
For example, we can imagine those python hooks send MoulinetteError, but currently we plan some change on MoulinetteError (see irina PR)... So include some code by that way could be trigger unwanted error after the PR be merged...

It could be ok for system hooks but for apps hooks it's quite difficult to manage with it... We might put a try catch around the load of the module ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment