search

ec-europa / rdf_entity

7 stars 9 forks source link

Provide proper upgrade path, despite this being in alpha #141

Open donquixote opened 2 years ago

donquixote commented 2 years ago

From the README.md:

As the module is in alpha, we're not providing any update path but we recommend to follow the next steps in order to update a server in production:

I see that this module is in alpha, but:

I assume that rdf_entity module is used primarily within the openeuropa ecosystem. Any oe site that wants to upgrade to D9 will have to deal with this upgrade.

The readme already offers some instructions, but I think the module needs to fully commit to supporting the upgrade. I had some problems, which I can open separate issues for. Here I am mainly asking to make this a goal we should aim for.

claudiu-cristea commented 2 years ago

@donquixote

but I think the module needs to fully commit to supporting the upgrade.

Unfortunately, that is technically impossible. Between 1.0-alpha16 and alpha17 we've split a big part of the module in a separate module. It's explained in the README why. But if you find a solution, I'd be happy to help

donquixote commented 2 years ago

@claudiu-cristea I read this, and I think we can accept that the upgrade will happen in multiple steps, with some manual steps. We just want to be sure that these instructions actually work, and treat any problem with these steps as a bug.

The current sentence in the README, "As the module is in alpha, we're not providing any update path", could discourage people from opening bug reports, as it implies that we don't really care.

So what would change:

donquixote commented 2 years ago

One such problem I opened here, https://www.drupal.org/project/rdf_entity/issues/3254048 I can open a clone of this issue here in github, if this is the preferred place. (also easier to create a PR)

donquixote commented 2 years ago

Btw seems like the code here in github is a bit out of date compared to drupal.org. So where is the correct place for everything?

claudiu-cristea commented 2 years ago

@donquixote use this https://github.com/idimopoulos/rdf_entity repo for work. The reason is that we cannot run tests on Drupal.org as we need to install Virtuoso in the pipeline which is not possible there. We used https://github.com/ec-europa/rdf_entity for development but at some point the namespace owner (ec-europa) disabled usage of Travis CI. Until finding a definitive solution where we're able to run tests, we use https://github.com/idimopoulos/rdf_entity

claudiu-cristea commented 2 years ago

In the README.md, instead of saying "As the module is in alpha, we're not providing any update path", say "due to technical constraints, the upgrade needs to be done in multiple steps.". This sounds like a very small change, but it changes the expectations that users can have.

Sure. Could you provide a quick patch?

We test the process, identify obstacles, and make sure it does work.

I wonder why rdf_skos was left behind, 8.x-1.0-alpha17 has been released in May 2019. We've successfully updated Joinup following that procedure

Any new issues about the upgrade process are addressed as actual bugs.

OK.

Add test cases for the upgrade? Not sure if this is possible..

It's not, as the main problem (see README) is the impossibility to update in a single step. The new module should be already installed when the update script starts to run, even it's an empty shell.

donquixote commented 2 years ago

Ok, good to know!

use this https://github.com/idimopoulos/rdf_entity repo for work.

So, issues here, but pull requests there?

Sure. Could you provide a quick patch?

One step at a time :)

I wonder why rdf_skos was left behind

Do you mean oe_content should require the 1.x version of rdf_skos instead of the 0.x version? Seems reasonable to me.

Add test cases for the upgrade? Not sure if this is possible..

It's not, as the main problem (see README) is the impossibility to update in a single step. The new module should be already installed when the update script starts to run, even it's an empty shell.

It would need some "inception"... The test would need to git clone, install with an earlier version, git checkout the new version, and install with a later version. I don't know if this is at all realistic.

donquixote commented 2 years ago

Atm, an obstacle I run into is this: system_post_update_entity_revision_metadata_bc_cleanup() fails on updb, because it cannot find \\Drupal\\rdf_entity\\Entity\\RdfEntitySparqlStorage, which seems to be stuck in the database as the entity class for rdf_entity. I checked the db dump, and found the class being referenced in the key_value table which contains the info about the entity type.

Perhaps we need an update hook to clear the entity type? Or perhaps the steps in our .opts.yml are insufficient? I can provide more info about this later, perhaps in a dedicated issue. But perhaps you already have an idea?

claudiu-cristea commented 2 years ago

Perhaps we need an update hook to clear the entity type? Or perhaps the steps in our .opts.yml are insufficient? I can provide more info about this later, perhaps in a dedicated issue. But perhaps you already have an idea?

We made this update in May 2019. It's possible that in the meantime this post update has been added in core. As I remember we didn't hit that. We've managed to do the update exactly as is mentioned in README. In fact README was written base on our update

donquixote commented 2 years ago

This PR by Marton! https://github.com/idimopoulos/rdf_entity/pull/4

donquixote commented 2 years ago

I wonder why rdf_skos was left behind, 8.x-1.0-alpha17 has been released in May 2019. We've successfully updated Joinup following that procedure

Do you mean oe_content should require the 1.x version of rdf_skos instead of the 0.x version?

Actually, oe_content:2.* uses rdf_skos:1.*, whereas oe_content:1.* uses rdf_skos:0.*. So, still not sure what you mean by "left behind".