Skip to content

Conversation

@agoscinski
Copy link
Contributor

@agoscinski agoscinski commented May 28, 2025

Embeds the docker-compose offered by firecrest-v2 into CI to run with real firecrest server.

Open questions:

  • Should we keep the mock? Then we need to run it in the CI and
    maintain it but maybe helpful for finding bugs. Then I would keept the
    workflow files
  • Fix v2 endpoint problems
  • Maybe use caching for docker compos up, before jpribyl/action-docker-layer-caching was used, not sure if it works in this case

@agoscinski agoscinski force-pushed the ci-f7t-v2 branch 4 times, most recently from 646030a to 89074cc Compare May 28, 2025 13:50
Embeds the docker-compose offered by firecrest-v2 into CI to run
with real firecrest server.

Open questions:
- [ ] Should we keep the mock? Then we need to run it in the CI and
  maintain it but maybe helpful for finding bugs. Then I would keept the
  workflow files
- [ ] Fix v2 endpoint problems
@khsrali
Copy link
Collaborator

khsrali commented Jun 2, 2025

Thanks @agoscinski !

Should we keep the mock? Then we need to run it in the CI and
maintain it but maybe helpful for finding bugs. Then I would keept the
workflow files

I'd vote for not having the mock anymore; maintenance overhead for little benefit it provides.
Instead, I suggest we run the CI-docker-image against all versions of firecrest that we support.
This way, while keep supporting higher versions incrementally, if a new bug appears, will be clear which function in version is causing that.

@agoscinski
Copy link
Contributor Author

agoscinski commented Jun 3, 2025

I suggest we run the CI-docker-image against all versions of firecrest that we support.

I checkout firecrest-v2 repo and use their docker-compose.yml file. The CI for firecrest-v1 would require a different way to set up firecrest as v1 does not provide the same resources. I mean we could try to support both v1 and v2 but given that the support of v2 is more important, I would focus von supporting v2 and then if there is time (which there never is) we could put back the code for v1. If you mean testing for multiple version of firecrest-v2, I would check which version is needed for sirocco CSCS CI and test for now only this, then other refs can be easily added later on.

@khsrali
Copy link
Collaborator

khsrali commented Jun 4, 2025

Ok if the setup is different, we can separate v1 as a branch that runs the tests only via mocked firecreset. This branch will be released but not maintained.

We then make our main branch compatible with v2 only. There we use the docker test and no mock.

with:
name: aiida-firecrest-pytests
flags: pytests
flags: pytests --.....
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
flags: pytests --.....
flags: pytests

@khsrali khsrali mentioned this pull request Jun 10, 2025
@khsrali
Copy link
Collaborator

khsrali commented Jun 20, 2025

merged via f574a4f

@khsrali khsrali closed this Jun 20, 2025
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