Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add aiida-sssp-workflow plugin #278

Closed
wants to merge 3 commits into from
Closed

Conversation

unkcpz
Copy link
Member

@unkcpz unkcpz commented Sep 19, 2023

No description provided.

@github-actions
Copy link

github-actions bot commented Sep 19, 2023

PR Preview Action v1.4.4
🚀 Deployed preview to https://aiidateam.github.io/aiida-registry/pr-preview/pr-278/
on branch gh-pages at 2023-12-04 14:01 UTC

@ltalirz
Copy link
Member

ltalirz commented Sep 19, 2023

@AhmedBasem20 , the new section in action - very nice

image

@unkcpz
Copy link
Member Author

unkcpz commented Sep 19, 2023

Yes, very nice! But since the container uses aiida-core==2.3.1 and my plugin requires aiida-core~=2.4.0 so it tries to install the old version of my plugin. 😂

@ltalirz
Copy link
Member

ltalirz commented Sep 19, 2023

Why is the container not up to date?

@unkcpz
Copy link
Member Author

unkcpz commented Sep 20, 2023

Two issues. First the aiida-core was not yet released after the new docker stack was built, so the tag that can be used is aiida-core-with-services:edge, the edge is from the main branch. It is better to use latest tag which points to the latest released version. Second, the time needed to run a test install is more than 2 times longer. Since the new docker stack will force us to wait for the services to start.

@unkcpz
Copy link
Member Author

unkcpz commented Sep 20, 2023

Just did a close look at why it takes so long for the new docker stack, the time to start the container is not too much but with the new version of aiida-core where python 3.8 was deprecated, lots of plugins just can not installed and takes very long to search for correct dependencies. Thus I think it makes sense to accept the test install time is long and I increase the CI timeout to 180 in #280. Let's go and discuss it there (and I'll merge this after #280).

@unkcpz unkcpz added the block label Sep 23, 2023
@ltalirz
Copy link
Member

ltalirz commented Sep 23, 2023

with the new version of aiida-core where python 3.8 was deprecated, lots of plugins just can not installed and takes very long to search for correct dependencies.

this is rather worrying to me... does this indicate it would be a good idea to delay this deprecation?

@unkcpz
Copy link
Member Author

unkcpz commented Sep 24, 2023

does this indicate it would be a good idea to delay this deprecation?

But this is already happened. I thought it won't be a problem but now I realize that if people check the aiida-registry page, it will give a false impression that a plugin cannot be used which does not support py3.9 yet but all fine in python3.8+aiida-core<2.4.0.

I'll compare and see how many plugins encounter such problems and see if we need to worry about it in here or aiida-core or just tell some plugins to migrate to python3.9 (if only 2 or 3 I think this is the easiest way to do so).

@unkcpz
Copy link
Member Author

unkcpz commented Sep 25, 2023

I realize another catch in terms of docker stack. The install test can actually override the aiida-core, which is why some plugins which not migrated to aiida 2.x still can pass the install test. I think this is not proper. Instead, I'd propose to be more aggressively pinning the aiida-core version as a strict dependency inside the docker image this will prevent plugins that are not compatible with aiida-core version of the container from falsely passing the test.

@ltalirz
Copy link
Member

ltalirz commented Sep 25, 2023

But this is already happened. I thought it won't be a problem but now I realize that if people check the aiida-registry page, it will give a false impression that a plugin cannot be used which does not support py3.9 yet but all fine in python3.8+aiida-core<2.4.0.

I'll compare and see how many plugins encounter such problems and see if we need to worry about it in here or aiida-core or just tell some plugins to migrate to python3.9 (if only 2 or 3 I think this is the easiest way to do so).

I suspect that most plugins will work fine with Python 3.9, but some may have outdated requirements on the python version.
If that is the case, we may want to wait with the upgrade
(actually, so far the only changes in aiida-core I saw that were specific to python 3.9 were related to typing, i.e. not relevant to running it. it may still work on python 3.8 just fine)

I realize another catch in terms of docker stack. The install test can actually override the aiida-core, which is why some plugins which not migrated to aiida 2.x still can pass the install test. I think this is not proper. Instead, I'd propose to be more aggressively pinning the aiida-core version as a strict dependency inside the docker image this will prevent plugins that are not compatible with aiida-core version of the container from falsely passing the test.

That is correct and I agree the aiida-core version should be pinned.

@unkcpz unkcpz removed the block label Dec 4, 2023
@unkcpz unkcpz force-pushed the master branch 6 times, most recently from 1b42871 to d2889c8 Compare December 19, 2023 02:46
@unkcpz
Copy link
Member Author

unkcpz commented Dec 19, 2023

Superseded by #299

@unkcpz unkcpz closed this Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants