-
Notifications
You must be signed in to change notification settings - Fork 0
Add celery worker #8
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
Conversation
…pdates into a single deploy job, triggered only on main branch pushes.
…ehensive project README, and a root `pyproject.toml` for shared configuration.
…nts across the API, worker, and E2E tests.
…sk timeout assertion, and introduce linting exclusions.
Pasarus
left a comment
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.
Would it make sense to add some unit tests notably the only thing I have noticed worth it, is likely that function I asked to go into a Util file.
I am guessing the reason we have a pyproject.toml at the root of the repo is for the e2e tests only? and the e2e needs both components in order to be worthwhile, is that correct?
This was actually to allow ruff to run on both services at the same time in CI |
…ted `model.py` and `utils.py` files, adding tests for the utility and updating `.gitignore`.
…om uvicorn access.
| steps: | ||
| - uses: actions/checkout@v3 | ||
|
|
||
| - name: Build and Run Tests |
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.
We probably also want to test the unit tests here right?
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.
Good catch
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 have added unit test jobs for both llsp worker and llsp api. The worker currently just contains a placeholder test as the functionality is not yet specialised enough to be tested. But this will ensure this same miss doesn't happen again in the future.
… test jobs, and add a placeholder test for worker unit tests.
Closes #
Description