-
Notifications
You must be signed in to change notification settings - Fork 6
🐛 Test "make build" of generated service #116
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed several breaking changes in this PR, especially since the goal seems to be simply testing make build. It seems like some important aspects of the design workflow might not be fully considered, and a few key components could be overlooked.
I suggest we take a step back and review how everything fits together. This way, we can collaboratively determine the best path forward before diving deeper into development.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are you removing all these bin? IMO this is essential. If you have another use case, you might have them optionally removed via the cc options
| key: simcore/services/{%- if cookiecutter.project_type == "computational" -%}comp{%- elif cookiecutter.project_type == "dynamic" -%}dynamic{%- endif -%}/{{ cookiecutter.project_name.lower().replace(' ', '-') }} | ||
| type: {{ cookiecutter.project_type }} | ||
| integration-version: 1.0.0 | ||
| integration-version: 2.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this value has to do with the ospar integration-version. It is defined on the osparc side.It defines how the catalog integrates this service. You cannot randomly change to another value!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WIP: work in progress
Current the cookiecutter tests only test that it is possible to generate a dummy project with the cookiecutter, but not whether such project is functional (e.g.
make buildcan be run on it). This can lead to issues like the one we had regarding the changes in the integration tooling (see here) not being detected until someone tries to use the cookiecutter for a real case scenario.This was said to not be possible because venv would need to be activated within the subprocess, which doesnt seem possible (or at least easy). Nonetheless, this only seems to be necessary to use the cookiecutter, not to test the "make" commands on the generated project.
The changes here show that it is possible to execute "make build" and, in the case of error (which I am currently getting), print the stderr to a file for debugging.
This is WiP - ideally will find what is giving this errors and manage to successfully build within the test, providing a real test of the cookiecutter ability to generate a functional service.
NB these changes should also be applied to the other cookiecutters once they work and get merged