Skip to content
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

[enh] Optimize app setting helpers #663

Merged
merged 3 commits into from Mar 9, 2019

Conversation

Projects
None yet
4 participants
@alexAubin
Copy link
Member

commented Feb 24, 2019

The problem

YunoHost/issues#1299

On an ARM board like RPi2, yunohost app setting takes 6~7 second to complete... Considering that an app usually interacts with 5~10 settings during an install, you may loose a whole damn minute just getting or setting settings...

Instead, doing this with a "small" python snippet can divide this time by ~10.

Solution

Have a small helper to manipulate the setting yaml files. Turns out it's not straightforward as there are some special cases (c.f. redirected_urls and redirected_regex) to handle to truly reproduce the yunohost app setting behavior.

PR Status

Partially tested on my machine

How to test...

Load and use the ynh_app_setting_{get,set,delete} in various case ;)

Validation

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

This comment has been minimized.

Copy link

commented Feb 24, 2019

Just out of curuosity, why is the yunohost app setting command taking so long in the first place?

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Feb 24, 2019

That's a good question, but I'm pretty sure it's more about yunohost commands in general than just this one specifically... I guess loading the whole moulinette + yunohost (all doing a lot of parsing + module loading) is long ... Would be nice to run some profiling some day to see if there's anything obvious we could do

@Psycojoker

This comment has been minimized.

Copy link
Member

commented Mar 1, 2019

Would be nice to run some profiling some day to see if there's anything obvious we could do

Already did and ... nothing really obvious came out of it :( (I remember doing some small optimisation but it wasn't dramatic)

Most of the time is spend in building the argument parsing tool with argparse (it's not even about the loading of the yaml/pickled file iirc) so I didn't end up finding something obvious to fix :[

@Psycojoker

This comment has been minimized.

Copy link
Member

commented Mar 1, 2019

I think the ideal solution would be to do a "yunolib" that extract auth independent business logic out of this code base and ship small CLI tools instead of inlining python into bash.

In the meantime that looks good enough I guess.

Show resolved Hide resolved data/helpers.d/setting Outdated

alexAubin added some commits Mar 1, 2019

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Mar 7, 2019

Proposing to merge soon

@JimboJoe
Copy link
Member

left a comment

LGTM

@alexAubin alexAubin merged commit 696d157 into stretch-unstable Mar 9, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@alexAubin alexAubin deleted the optimize-app-setting-helpers branch Mar 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.