How to support scripts that write is_active in the new schema?

[manuelma]

We are in the process of deprecating tool, feature, and method, and use entity_alternative instead. In short, entity_alternative holds triplets (entity, alternative, activity) telling which entity is active in which alternative. Applying the scenario or alternative filter would filter out all the entities that have activity 'false' for the winning alternative.

Because we know (?) that the only application of tool, feature, and method out there is to control entity activity, we are fine. It is possible to migrate DBs from the old to the new schema.

However, we cannot possible migrate scripts that import tool/feature/method stuff as well as is_active parameter values. The question is how do we handle this. We can't ask users to migrate their scripts before starting to use 0.8. We need 0.8 to work with everything the user currently has, to give them time to migrate their stuff.

So I see two options:

1. Keep tool/feature/method together with entity_alternative. In this case the user can keep setting tool/feature/method and is_active as usual and it will work. They can also start using entity_alternative if they want. Both things will work.
2. Get rid of tool/feature/method/is_active and migrate everything into entity_alternative. If the user tries to set tool/feature/method stuff, we just ignore it. If the user tries to set is_active, we convert it to a corresponding record in entity_alternative under the hood.

Option 1 is more complete, but a lot heavier. Option 2 is a bit more loose (we need to hardcode the translation from is_active into entity_alternative in the api, at least for a while) but allows us to get rid of a lot of stuff, which is never bad.

Which one do you prefer? Are there any other options?

@jkiviluo @DillonJ @soininen @PekkaSavolainen @PiispaH @suvayu ? Please tag anybody else who might have an opinion.

[jkiviluo]

Maybe it's more important for us to move fast and possibly break a few things than to ensure everything will keep working. It's hard to say how many outsiders are using SpineDB_API, but I suspect that not that many and we know some of them (77 unique cloners of Toolbox in the past 2 weeks, most of them probably just users of SpineOpt and FlexTool). If we would ditch fool/feature/method directly, we should then run an information operation (e-mail those we know and advertise in GitHub / code). This should include a more explicit warning when new Toolbox version tries to migrate a DB from 0.7 to 0.8 -- "Migrate only once all of your code using spineDB_API works with the new database version (tool/feature/method has been replaced by entity_alternative)". The instruction would be to 1) migrate a test database and then 2) fix their scripts to work against the migrated database in a separate branch and 3) make the jump with real databases only once everything works. We could also automatically leave behind a v0.7 version of the database (and inform about it).

But if this is not palatable for others, then I would rather do option 2 from Manuel's list (or something else that keeps the workload on our side relatively small - we have lot of great things we need to get to implement).

[manuelma]

Thank you @jkiviluo

I was thinking that yet another option is to deal with is_active at SpineInterface level. Concretely, SpineInterface would filter out entities that have is_active false. This is low-effort on us and might ease migration on users of the Julia packages.

[jkiviluo]

I was thinking that yet another option is to deal with is_active at SpineInterface level.

Wouldn't we then need another fix for those using SpineDB_API directly? That would need to be option 1 - otherwise SpineInterface could not use is_active.

[DillonJ]

My thoughts on this are that the reasoning around tool-feature-method are pretty sounds since is_active is the only application of it. So option 2 or the SpineInterface option seem like the right way to go.

I guess the SpineInterface idea is that if we get rid of tool-feature-method, then is_active is just another parameter in a SpineDB and SpineInterface would treat it specially. Maybe I have misunderstood the issue?

Edit: I think the choice is either dealing with is_active at DB_API level independently of tool-feature-method or at SpineInterface level... if we deal with it at SpineInterface level, then some users that use the api directly would be impacted.

[soininen]

I feel we should not put too much effort on backwards compatibility that would benefit only a few users --- if any. If I was allowed to go with just a gut feeling, I would choose option 2 with the change that we should signal a warning instead of silently ignoring anything. I think that option 2 is the better compromise between developer burden and user inconvenience.

[jkiviluo]

If the user tries to set is_active, we convert it to a corresponding record in entity_alternative under the hood.

It sounds like option 2 then. In that case we should probably also deprecate this under the hood thing for is_active at some point as it's quite hacky.

[DillonJ]

Once thing that's nice about is_active is that it's a parameter and is quite "portable". For example, I can copy/paste from the parameter pane and I get everything, including the activity of the entity. Now there will be two places to look to get all the relevant details about an entity. Perhaps the new entity pane will help (is this planned for 0.8?). Even then, it's still another place to look. An advantage of having the two methods side by side for a while will perhaps allow us to get comfortable and maybe optimise the UI to make it work just as well

[suvayu]

2. Get rid of tool/feature/method/is_active and migrate everything into entity_alternative. If the user tries to set tool/feature/method stuff, we just ignore it. If the user tries to set is_active, we convert it to a corresponding record in entity_alternative under the hood.

I hope my feedback is not too late. 2 is definitely preferred, as for how to implement it. The general recommendation is something like this:

1. change the old API to read-only, so setting fails, but accessing is_active works.
2. when setting fails, give a deprecation warning, with a resolution, i.e. point to docs showing how to migrate to entity_alternative
3. while reading is_active still works, it should also emit the same deprecation warning as 2
4. the deprecation warning should also include something along the lines of "From release version X.Y this will be an error and your code will not work"

Now about where to implement, I agree with Juha, the fix should be as "low-level" as possible, so in spinedb_api. This way there are fewer chances of missing a use-case.

[manuelma]

Thanks for all the input, that's great!

So here is the plan to implement option 2:

1. In the migration script that introduces entity_alternative, transform is_active parameter_value rows into entity_alternative rows.
2. Whenever the user tries to add or update is_active parameter_value rows via the API, add or update a corresponding entity_alternative row instead (while issuing a deprecation notice).

For step 2 I would need to know what are the possible values for is_active in FlexTool, is it true/false as in SpineOpt, or something else? It would be good to have a FlexTool DB to test too. @soininen @jkiviluo can you maybe provide one?

[soininen]

It would be good to have a FlexTool DB to test too.

You can download Init.sqlite from here

[manuelma]

I am missing something or the is_active parameter value list only has one value (yes)? How does it work then?

[soininen]

I am missing something or the is_active parameter value list only has one value (yes)? How does it work then?

You're not missing anything. Apparently it works this way, too. Or I hope it does :) I guess FlexTool users don't need the option to turn is_active 'off'.

[manuelma]

Thank you @soininen !

[jkiviluo]

Is it too difficult to read the tool/feature/method in the migration in case someone is using another word for is_active? I don't know of any such case, but it's possible.

[manuelma]

In the migration is possible but not in step two above ("whenever the user tries to add or update is_active parameter_value rows via the API, add or update a corresponding entity_alternative row instead") because the tables are already gone...

[manuelma]

I guess an option is not to drop the tool/feature/method tables just yet but keep them 'internal' so to say, just so we can handle all cases gracefully. I don't dislike that at all - we can drop them in a further migration somewhere down the line?

[jkiviluo]

Sounds like a nice thing to do to be on the safe side.

[manuelma]

This is done and unit-tested but some human testing might reveal some problems I believe. Will close so people can open new issues for specific problems when the time comes.

[manuelma]

Just a summary of what was done:

1. A migration script that converts tool-feature-methods into entity_alternative rows. This covers the case where the user has an older DB with is_active parameter values.
2. A system so when the user enters is_active parameter values and then commit changes, those parameter values are automatically converted into entity_alternative rows. This covers the case where the user creates a new DB so it doesn't get caught by the abovementioned migration script.