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

enable a redis container in github workflow and configure test_service.ipynb to use it #65

Closed
3 tasks done
hagrid67 opened this issue Dec 6, 2023 · 2 comments · Fixed by #107
Closed
3 tasks done
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@hagrid67
Copy link
Contributor

hagrid67 commented Dec 6, 2023

Objectives
Re-enable test_service.ipynb in tox notebooks to test the evaluation service

Todos

  • enable a redis container for the notebooks workflow
  • pass the redis container host(?) to the notebook in an environment variable redis_ip
  • uncomment the test_service.ipynb

Minimal testing requirements
N/A - this change enables an existing test to run

Additional context
checks.yml notebooks have recently been enabled by Jeremy / @hagrid67 in PR #61
@fnberta would you mind taking a look at enabling redis in the workflow please? (Or I'll take a look in a few days)

@hagrid67 hagrid67 added the enhancement New feature or request label Dec 6, 2023
@fnberta
Copy link
Contributor

fnberta commented Dec 6, 2023

Yes, I can look into it. But do we really want to require a redis instance to be running just to run the checks in our build pipeline?

@hagrid67
Copy link
Contributor Author

hagrid67 commented Dec 7, 2023

It seems that running Redis in github CI is well established.

I think we have plans to offer the evaluation service as a key feature of flatland, so that people can publish their benchmarks. We discussed at the last dev meeting starting work by the end of the year; Manuel is doing some planning. So I'd suggest that it's worth putting the effort into building some tests around it. The existing evaluator uses redis; this comes from the AICrowd architecture.

The notebook is one I built 2-3 years ago to run a simple test to start an evaluator service and then run a client against it. It "manually" starts and stops processes in the background so I'm sure it could be improved; eg we might want to run Redis in one container, and the evaluator in another, and then run the client in the foreground.

Until then, it seems fairly low effort to get this existing test working on github CI. It seems to run OK on my local Ubuntu 22.04 with redis running.

@chenkins chenkins assigned chenkins and unassigned fnberta and hagrid67 Jan 9, 2025
@chenkins chenkins added this to the 4.0.4 milestone Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants