-
Notifications
You must be signed in to change notification settings - Fork 70
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
Requirements for mlbackend_python tests missing #9
Comments
See https://github.com/danpoltawski/moodle-docker/issues/29 ideally, I don't think thus should be part of this image as it seems a better fit for a dedicated container. |
Related issues: danpoltawski/docker-moodle/#9.
Related issues: danpoltawski/docker-moodle/#9.
Related issues: danpoltawski/docker-moodle/#9.
Hi, I've been looking at this for some time now. I've set up a new image with the python package installed. It can be run from the local filesystem setting $CFG->pathtopython to docker run --rm -ti dmonllao/moodle-mlbackend:test python (dmonllao/moodle-mlbackend:test is just this: https://github.com/dmonllao/moodle-docker-mlbackend) but the docker command is not accessible from the webserver container and I imagine that installing docker on webserver is not nice. Our mlbackend does not have any REST API nor something like that and it is just a CLI command executed from the webserver (changing this is a separate topic and it should part of the roadmap now that I have time for analytics stuff) but, in the meanwhile, I don't have any other proposal than to install python and the moodlemlbackend pip package in https://github.com/moodlehq/moodle-php-apache/blob/master/Dockerfile although I haven't tried if php:7.1-apache-stretch comes with apt-get. I am 100% open to suggestions. |
I think that, it is not worth spending more time in the new-container approach until the python ml backend does not allow execution from a remote server. I've modified both moodle-php-apache and moodle-docker so we can execute python mlbackend tests using docker: I didn't set $CFG->pathtopython into if (getenv('MOODLE_DOCKER_PHPUNIT_EXTRAS')) because we set PHPUNIT_LONGTEST to true and because python dependencies will be installed anyway in moodle-php-apache. The patch has been tested in my local machine. I built new moodle-php-apache tags and I updated moodle-docker's base.yml to point to my local image and tag. Analytics tests that involve using the python backend to generated predictions passed without skips (although including an annoying and useless warning from one of the moodlemlbackend package dependencies)
|
+1 to create a PR about this issue! |
Thanks, I've created moodlehq/moodle-docker#90 and #35 |
Hi, the patches above install the python ml backend in the web server. Moodle will be able to communicate with the python backend once https://tracker.moodle.org/browse/MDL-66004 is installed. I set up an interim docker image for it in https://hub.docker.com/r/dmonllao/moodle-mlbackend-python. It would be great if someone from iteam could confirm that the proposed approach is enough to satisfy the requirements to deploy it in iteam's infrastructure. |
It'd be great to include the python library required to run the mlbackend_python tests.
The text was updated successfully, but these errors were encountered: