Problem/Motivation

My sites are broken due to the missing service as we don't have the key module installed.

Issue fork ip_info-3516274

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jurgenhaas created an issue. See original summary.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Related issues: +#3516248: Integrate the Drupal module key
jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented

The problem is that this module introduces a dependency on the key module but there is no upgrade path for existing sites. And that would be pretty difficult anyway, as enabling the required key module fails if the cache needs to be rebuild before then as the ip_info.base service requires a service from the key module, but that doesn't exist.

Also, all existing key config is now lost. There should have been an update hook which turns the existing config into key storage.

Last but not least, the ip_info.settings config stays available. This should be deleted by the update hook.

antonín slejška’s picture

antonín slejška commented

Hi Jürgen,
thanks for the analysis. There is a deploy hook, which should update the ip_info.settings. But I forget to add the update hook, which should enable the Key module.

antonín slejška opened merge request !6

antonín slejška’s picture

antonín slejška commented
Status: Active » Needs review

Here is the merge request. Could you please review it?: https://git.drupalcode.org/project/ip_info/-/merge_requests/6/diffs

antonín slejška’s picture

antonín slejška commented
Status: Needs review » Fixed
jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented

There is a deploy hook, which should update the ip_info.settings.

Hmm, that's new to me that such a deploy hook exists. Do you have some documentation what's executing that hook?

the update hook, which should enable the Key module.

That's still problematic. It may work in some cases but not in others because the new service declaration already refers to a non-existent service from the key module before it can get enabled.

antonín slejška’s picture

antonín slejška commented

The deploy hook https://git.drupalcode.org/project/ip_info/-/blob/1.x/ip_info.deploy.php goes through all ip_info keys/tokens in the config ip_info.settings or in the settings.php and creates new keys in the Key module. They relations to the keys are then usedin the ip_info.settings config instead of the keys.

I suppose I have to add the install hook also to the ip_info.deploy.php file. The Key module is installed in the post_update hook. But in the drush deploy command (https://www.drush.org/13.x/deploycommand/) runs config-import after it. This deactivates the Key module. It should be activated in the deploy hook.

jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented

My question still is, where is that deploy hook coming from? I mean, what is executing this hook?

jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented

And again, which ever way, you should not introduce a new dependency and already use it in a service. Whatever your users are doing, they will most likely not be able to install that new dependency because the site breaks.

antonín slejška’s picture

antonín slejška commented

The hook is executed by drush deploy:hook command.

The problem in the ip_info.deploy.php is, that there is the use statement
use Drupal\key\Entity\Key;
But the module is at the moment not installed.

It is the first time, that I add a new dependency in a contrib module. It looks like I have to just add the dependency without using it and in the following version of the module to add the code using the dependency. I have no idea, how I can solve the problem now.

antonín slejška’s picture

antonín slejška commented

Maybe, I can create a new version 1.1.2 where the new dependency is installed.
And then I create the version 1.1.3 where the Key module is used.

jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented

The hook is executed by drush deploy:hook command.

OK, so I have no idea how many people will actually get that. It's not an official way to update modules, that should only rely on core hooks.

The way you describe the update process in two steps for the future sounds about right. As for this time, I'm not sure you need to change that again. There are just 25 reported sites using the module so far, and I have fixed mine manually. But I can't tell about the others.

antonín slejška’s picture

antonín slejška commented

The 1.1.3 should work correctly.

It is necessary to run the drush deploy:hook or drush deploy command!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.