Skip to content
This repository has been archived by the owner on Aug 25, 2022. It is now read-only.

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

Open
donquixote opened this issue Dec 13, 2021 · 11 comments
Open

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

donquixote opened this issue Dec 13, 2021 · 11 comments

Comments

@donquixote
Copy link

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:

  • oe_content (stable) requires rdf_skos (0.x).
  • rdf_skos (0.x) requires rdf_entity (alpha).

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
Copy link
Contributor

@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
Copy link
Author

@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:

  • 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.
  • We test the process, identify obstacles, and make sure it does work.
  • Any new issues about the upgrade process are addressed as actual bugs.
  • Add test cases for the upgrade? Not sure if this is possible..

@donquixote
Copy link
Author

donquixote commented Dec 13, 2021

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
Copy link
Author

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
Copy link
Contributor

claudiu-cristea commented Dec 13, 2021

@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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

This PR by Marton! idimopoulos#4

@donquixote
Copy link
Author

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".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants