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

Refactor updater and maybe installer #5985

Open
tsteur opened this issue Aug 12, 2014 · 5 comments
Open

Refactor updater and maybe installer #5985

tsteur opened this issue Aug 12, 2014 · 5 comments
Labels
Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.

Comments

@tsteur
Copy link
Member

tsteur commented Aug 12, 2014

In the past months we sometimes had troubles with extending the Updater / Installer.

  • For instance I had to add many hacks just to make dimensions installable via the Updater.
  • For instance it is not possible to disable a plugin or an option before calculating the queries to execute.
  • We have troubles removing files during update On update, automatically delete from filesystem all Piwik files that are not in the latest release #5936
  • It is expensive in terms of time to check whether an update is available each time (at least for ColumnsUpdater). I am using some caching currently to reduce the time again
  • We have to run the updater twice sometimes for instance to activate the plugin in first update and to install the newly detected dimensions in the second run
  • We also should run the updater in two steps and first try to install all plugins and then perform all needed updates if the recorded component version is lower than the current used version see Unknown column 'use_12_hour_clock' on update from 2.15.0 to 2.15.1-b2 or later #9666 (comment)
  • We always should as well check the recorded component version when deciding whether we need to perform an installation. This is a use case see Unknown column 'use_12_hour_clock' on update from 2.15.0 to 2.15.1-b2 or later #9666 (comment) when deleting config.ini.php and then updating Piwik. Problem is when there are not only database changes but also local changes (eg modifying piwik.js). Local changes would always need to be executed while database changes only might need to be executed when recorded component version in database is lower. Maybe we can separate those two use cases in the installer with different methods or so.
  • If someone executes all the SQL updates manually the user should still go through the updater but the already executed updates should be ignored. All other actions/updates should be still performed
  • some files are sometimes not correctly copied or caches not invalidated etc (we see this problem often in issues here)
  • The updater does not remove created option entries when a dimension or a plugin is uninstalled. See http://forum.piwik.org/read.php?2,120959 . It is currently hacked in plugin manager but this one should actually not know anything about options and version.
  • Queries executed by a plugin install are not shown in core:update dry-run
  • There are some users who execute SQL queries manually. They still have to press "Update" afterwards in the UI. There we execute all SQL queries again. Most of them will fail and ignored but some queries are executed multiple times (eg INSERT INTO etc). See On updates, the version number is not updated when manually executing the SQL queries #7840 . The updater should be built in mind with users who execute SQL queries manually and not insert each SQL query a second time.
  • Optional possibility to run SQL queries in advance before updating the codebase for some queries when a migration script does not include any PHP update and only SQL update.
  • ...

As the platform grew we simply have more and more requirements to the updater but maybe also to the installer. With the knowledge we have today we could maybe improve the code of the updater and add some more features to make updates more flexible.

@mattab
Copy link
Member

mattab commented Jan 12, 2015

in #6966 a bug was reported that may be caused by the fact that during Upgrades some calls to activatePlugin may fail for example, when config file not writable. When the activatePlugin call fails in this way currently it is within a try/catch block -> it silently fails and doesn't report error to user.

Instead of silently ignoring the activatePlugin, we should WARN and explain user he should activate plugin manually. Maybe we need a new system in Updater to register commands like Plugin activation and Plugin deletion. Ideally thse commands would be also written during the dry-run so that the dry-run would indicate almost all modifications done during an update.

@diosmosis diosmosis self-assigned this May 18, 2015
@diosmosis diosmosis modified the milestones: 3.0.0, Mid term May 19, 2015
@diosmosis diosmosis removed their assignment May 22, 2015
@mattab mattab modified the milestones: Short term, 3.0.0 Aug 13, 2015
@mattab mattab added the Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. label Jun 23, 2017
@mattab
Copy link
Member

mattab commented Sep 18, 2017

Another issue (which is rather a "New feature" and should probably be in a separate issue) is that the upgrader should handle better the case when a plugin installation or upgrade triggers a large schema change, which times out in the browser. In some cases when the upgrade times out (or is it maybe in all cases?!), Piwik thinks that the plugin was successfully installed/upgraded although the schema changes were not fully done. This creates serious bugs such as matomo-org/plugin-MarketingCampaignsReporting#13 (comment)

There is a workaround for this issue, which is to uninstall & re-install the plugin, and run the ./console core:update in the console so it does not time out.

@tsteur
Copy link
Member Author

tsteur commented May 14, 2018

Just added another point to the list it would be very helpful if we were sometimes able to run SQL queries for updates in advance before updating the codebase. We would then also want to set a specific update script as "recorded" so it won't be executed again later. This works when the SQL query does not affect the product when using still an older codebase. This is something for advanced users only and we wouldn't make this probably much visible or at all. But we have to have the ability to run certain updates in advance and mark a certain update file as recorded. Also without getting the message "you are running an outdated codebase version of matomo with a newer DB version"

@diosmosis
Copy link
Member

What about instead of tying migrations to update versions, they are tied to a creation timestamp which serves as both ID & indication of order? Would have two benefits: one in that you could create a command to run/undo ('undo' in the future at least) any set of migrations by it's ID, and would mean if you created an Update, you don't have to also change the matomo version.

@tsteur
Copy link
Member Author

tsteur commented May 14, 2018

Yes had something like this in mind too. We might even want to think about not mixing DB migrations with PHP migrations (eg config file etc) but that's a different topic and might not always be possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Projects
None yet
Development

No branches or pull requests

3 participants