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

Support Python Machine learning backend #316

Merged
merged 2 commits into from
Feb 27, 2025

Conversation

HuongNV13
Copy link
Contributor

The patch adds support for Python Machine learning backend so we can run PHPUnit for core_analytics_testsuite

@HuongNV13 HuongNV13 force-pushed the support_python_mlbackend branch 2 times, most recently from debbdd6 to e036b75 Compare February 26, 2025 04:17
@lameze
Copy link
Contributor

lameze commented Feb 27, 2025

Hi @HuongNV13 thanks for adding the support for Python machine learning backend.

Overall everything looks good, I just have a few questions:

  1. Do we also need to add the support for Behat? If yes, maybe we should rename this $MOODLE_DOCKER_PHPUNIT_MLBACKEND to something less specific to PHPUnit?
  2. If this is specific to PHPUnit should it work with this MOODLE_DOCKER_PHPUNIT_EXTRAS and be placed in that if block alongside all other extras (solr, redis, ldap and etc)?
  3. Do you think it worth adding that new env. variable to the list of Environment variables?
  4. I've had a look on the failed builds related to this PR and they seem unrelated, could you please double check?

Anyway, great job legend.

Cheers

 - PHPUnit 11.4 compatible
 - Remove Oracle Database test for Moodle 5.0
@HuongNV13 HuongNV13 force-pushed the support_python_mlbackend branch from e036b75 to 1bc4d73 Compare February 27, 2025 06:49
@HuongNV13
Copy link
Contributor Author

Thanks, @lameze for taking a look at this one.
Yes, this one is for both Behat and PHPUnit, so I have renamed it to MOODLE_DOCKER_MLBACKEND
I also added a new environment variable to the readme file.

The failures are not related to this patch, we might need to update the MSSQL YML file to make the tests back to green.

Thanks,

@lameze
Copy link
Contributor

lameze commented Feb 27, 2025

Hi @HuongNV13 thanks for making those changes.

Given this can be used by Behat and PHPUnit, then it makes sense to have a separated section in config.php instead of adding it to the phpunit-external-services.yml.

Happy to get this merged now.

Cheers,

Simey

@lameze lameze merged commit 1c4c1a4 into moodlehq:main Feb 27, 2025
111 of 133 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants