Skip to content

Conversation

@jwwwb
Copy link
Contributor

@jwwwb jwwwb commented Apr 11, 2022

Describe changes

I changed the fresh stack-store fixture to spin up the FastAPI service using the multiprocessing module included with python, instead of the zenml specific local service daemon, which uses the ZEN_GLOBAL directory to store its state. This should allow the unit tests to run in parallel.

Pre-requisites

Please ensure you have done the following:

  • I have read the CONTRIBUTING.md document.
  • If my change requires a change to docs, I have updated the documentation accordingly.
  • If I have added an integration, I have updated the integrations table.
  • I have added tests to cover my changes.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Other (add details above)

@github-actions github-actions bot added internal To filter out internal PRs and issues bug Something isn't working labels Apr 11, 2022
@schustmi
Copy link
Contributor

I think the PR is fine as-is from a code perspective, I do have one alternative suggestion though (which might not be possible or feasible in a reasonable timeframe): As I imagine services will become an integral part of many ZenML integrations/features and will therefore appear in many tests, does it make sense to make this switch from using the daemon code in the service to multiprocessing by patching the LocalService for tests somehow? (Maybe @stefannica has an opinion on this as well)

@jwwwb jwwwb marked this pull request as ready for review April 14, 2022 12:26
@jwwwb jwwwb merged commit 0109027 into develop Apr 19, 2022
@jwwwb jwwwb deleted the bugfix/service-start-failure-in-unit-tests branch April 19, 2022 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working internal To filter out internal PRs and issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants