search

aiidalab / aiidalab-home

AiiDAlab Home App
MIT License
5 stars 9 forks source link

Strict dependencies check for aiida-core #128

Closed unkcpz closed 1 year ago

unkcpz commented 1 year ago

fixes https://github.com/aiidalab/aiidalab/issues/318

The compatibilities only checked for the installed app. If the user going to install the app that would potentially override the aiida-core version the installation will fail in AppManager but the user is not notified before they trigger the installation procedure. In this PR, the check for the to-be-installed app is activated when the version is selected and will block the user to install the app that has incompatible "core" dependencies. The "core" dependencies now are only aiida-core. The method _check_strict_dependency is used to check that the aiida-core version of the container is not overridden by the app.

If the "core" dependencies are not incompatible, the dependencies to be installed or updated will show in the info notes to remind the user what will be overridden.

unkcpz commented 1 year ago

To test with this PR, you can clone the repo and ln the /home/jovyan/app/home to the new folder.

unkcpz commented 1 year ago

Thanks @danielhollas, you ask very valuable questions.

Does this mean that no changes are needed in the aiidalab repo?

When I started, I plan to put functions in aiidalab and only use aiidalab-home as the viewer to reflect the change but found it is hard to implement since there is back and forth traitslet change between two packages. I put all function in aiidalab-home to make me easy to code. After I look the changes again, I think the controller part need to move to aiidalab package, and it is now doable. I'll try to refactor it a bit to make it more clear.

Will this also allow us to solve the spurious "Update available", if the new version is incompatible? Perhaps that's for a follow up PR?

No, I didn't take it into account. I test on the latest docker stack to show that downgrade to old version is forbidden. But as you said, this change is more for forbidding user to upgrade app and override aiida-core. I'll check it again.

We will need to backport this to the old docker stack image. We need to find out if the current aiidalab-home is still compatible or if there were any breaking changes in the meantime.

I don't think there will be breaking changes but as mentioned above this need to backport and I'll test it.

unkcpz commented 1 year ago

I address the variables naming suggested by @danielhollas and close this one with replaced it by #130.