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

Add ynh_script_progression #634

Merged
merged 1 commit into from Feb 10, 2019

Conversation

Projects
None yet
2 participants
@maniackcrudelis
Copy link
Contributor

commented Jan 28, 2019

The problem

I know that this helper is brand new, 2 days only, but there's no real danger with it and I really would like to see it widely used by all apps. Especially official ones.

Solution

Add the wonderful new helper ynh_script_progression :D

PR Status

Ready to be reviewed.

Validation

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

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2019

@alexAubin

This comment has been minimized.

Copy link
Member

commented Feb 4, 2019

About the usage of this script : please note that it might not be relevant for backup and restore scripts !

For instance, the way backup happens is to run the backup script only to get the list of files to backup (modulo the mysql dump) but not to copy them yet. So the bulk of the execution time is expected to be after running all those script. In that case, what the use might see is :

  • launch the backup thing
  • all apps write progress bar very quickly
  • everything seems to be stucked (because no feedback)
  • user might wonder if backup is still going on or system is blocked somehow...

And a similar thing (in reverse) happening when restoring

So I would not recommend using this helper during backup (though I havent tried to confirm the behavior). If there's a progress bar to be implemented for this, it should be implemented in the core during the actual copy I think...

@maniackcrudelis

This comment has been minimized.

Copy link
Contributor Author

commented Feb 5, 2019

There's actually other steps in backup and restore, especially restore script with reinstallation of dependencies, restart of services and other things like that which take time.
However, we should indeed finish the backup script with a message to inform that it's not completely done.

@alexAubin
Copy link
Member

left a comment

So, didn't really tested this but I saw it in action and it looks pretty cool 👍

@alexAubin alexAubin added this to the 3.5.x milestone Feb 5, 2019

@alexAubin alexAubin merged commit 55f19c7 into stretch-unstable Feb 10, 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 add_ynh_script_progression branch Feb 10, 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.