Skip to content

Conversation

@kumy
Copy link

@kumy kumy commented Nov 21, 2025

Description

Added a check to verify if the database structure is installed. So the installer can run when the config file is pre-configured, or in conjunction with plugin-EnvironmentVariables.

We're trying to deploy a matomo as a stateless container, for that we preconfigure the config.ini.php using docker configs., But as the the username is set in the config that prevent the installer from running the initial wizard.

Checklist

  • [✔/✖/NA] I have understood, reviewed, and tested all AI outputs before use
  • [✔/✖/NA] All AI instructions respect security, IP, and privacy rules

Review

Added a check to verify if the database structure is installed. So the installer can run when the config file is pre-configured, or in conjunction with plugin-EnvironmentVariables
@sgiehl
Copy link
Member

sgiehl commented Nov 24, 2025

@kumy Wouldn't it be enough if you set installation_in_progress=1 into the config file? 🤔
For a logical point of view it seems correct to add a table check to that method. For a performance point of view I wonder if that would be wise, as it would check for existing tables with every call of the method, which might even happen multiple times per request. So if we want to keep it, we might need to cache that I guess.

@kumy
Copy link
Author

kumy commented Nov 24, 2025

@sgiehl having to do installation_in_progress=1 would mean we have to commit one version of the config.ini.php containing it, then deploy, then fill the form, recommit to remove the installation_in_progress=1 and then redeploy. It's technically doable but cumbersome.

I can look to add a cache on that check, any tip on an example somewhere else in the base code?

@sgiehl
Copy link
Member

sgiehl commented Nov 24, 2025

The installation process should normally modify the config file anyway (and also remove installation_in_progress=1 in the end), also various actions within Matomo might cause a change to it. Having it non-writable might limit certain parts of Matomo.

@kumy
Copy link
Author

kumy commented Nov 24, 2025

The installation process should normally modify the config file anyway

yeah we saw that :/ and fight a bit with it. On first time install, we only need to do a service restart (not a full chain of commit + deploy) to restore the config as we expect it to be.

Having it non-writable might limit certain parts of Matomo.

This is exactly what we tried to achieve in our "GitOps world". What issue do you think we will encounter?

I quickly had a look at caching and found usage of PiwikCache::getTransientCache() around in the file, I crafted this untested (as I'm not at work right now) code. Is this the right track?

        // Check the database structure
        $cacheId = 'isInstalled';
        $cache   = PiwikCache::getTransientCache();
        if ($cache->contains($cacheId)) {
            return $cache->fetch($cacheId);
        }
        if (!DbHelper::isInstalled()) {
            return false;
        }
        $cache->save($cacheId, true);
        return true;
    }

@sgiehl
Copy link
Member

sgiehl commented Nov 24, 2025

This is exactly what we tried to achieve in our "GitOps world". What issue do you think we will encounter?

There are a couple of configurations and actions within the Matomo admin that may result in changing config flags in the config. If writing the config file doesn't work it may display errors accordingly.

PiwikCache::getTransientCache()

The transient cache is only for that one request, but it won't prevent other requests to do the table checks again. We may need to check if there is a better way to achieve that in general. 🤔

I guess what you actually would need is a command to execute the installation (or parts of it) in general, as requested in #10257, right?

@kumy
Copy link
Author

kumy commented Nov 24, 2025

Reading #10257 and effectively the first posts look close to what we need... Will continue reading... Thanks

We may need to check if there is a better way to achieve that in general.

I'll not be the best one to do that :/

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants