-
-
Notifications
You must be signed in to change notification settings - Fork 215
[MNT] Dockerized tests for CI runs using localhost #1629
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
base: main
Are you sure you want to change the base?
Conversation
Locally, MinIO already has more parquet files than on the test server.
Note that the previously strategy didn't work anymore if the server returned a parquet file, which is the case for the new local setup.
This means it is not reliant on the evaluation engine processing the dataset. Interestingly, the database state purposely seems to keep the last task's dataset in preparation explicitly (by having processing marked as done but having to dataset_status entry).
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1629 +/- ##
===========================================
+ Coverage 52.75% 69.74% +16.98%
===========================================
Files 36 36
Lines 4333 4333
===========================================
+ Hits 2286 3022 +736
+ Misses 2047 1311 -736 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
|
@geetu040 Doesn't #1630 close #1614 ? If services PR 13 and 15 add the v1 and v2 apis to the services repo, and #1630 allows them to be used as a Edit: I think the only thing I can do in addition to Peter's PR is run the setup during CI, so I close this PR and open a new one stacking on #1630 and adding this additional functionality. |
#1630 adds variable Point of this PR is to get the docker services running in the CI.
Keep this PR, how it already is pointing to |
geetu040
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.
- merge 1630 into this PR, mention in PR description that it's stacked over
- then use local-host api in
TEST_SERVER_URL - remove the marker "uses_test_server" in
test.yml
and let's see what happens in the CI
geetu040
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.
where do you set the TEST_SERVER_URL from env in tests?
I was going to remove that given it is not needed right now, |
| working-directory: ./services | ||
| run: | | ||
| sudo systemctl stop mysql.service | ||
| docker compose --profile rest-api --profile minio --profile evaluation-engine up -d |
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.
why do we need to run evaluation-engine?
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.
add env var
OPENML_USE_LOCAL_SERVICES: "true"
| run: | | ||
| timeout 180s bash -c 'until curl -sSf http://localhost:8000/api/v1/xml/data/1 > /dev/null; do | ||
| echo "Server still booting... retrying in 5s"; | ||
| sleep 5; | ||
| done' | ||
| curl -I http://localhost:8000/api/v1/task/1 |
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.
| run: | | |
| timeout 180s bash -c 'until curl -sSf http://localhost:8000/api/v1/xml/data/1 > /dev/null; do | |
| echo "Server still booting... retrying in 5s"; | |
| sleep 5; | |
| done' | |
| curl -I http://localhost:8000/api/v1/task/1 | |
| run: | | |
| echo "Waiting for API to become available..." | |
| timeout 30s bash -c 'until curl -sSf http://localhost:8080/api/v1/task/1 > /dev/null; do sleep 2; done' | |
| echo "Response:" | |
| curl http://localhost:8080/api/v1/task/1 |
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.
Similar to what there was earlier, changed it the previous commit and was going to revert it after CIs
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.
Similar to what there was earlier, changed it the previous commit and was going to revert it after CIs
| yield | ||
| return | ||
| openml.config.server = "https://test.openml.org/api/v1/xml" | ||
| openml.config.server = f"{openml.config.TEST_SERVER_URL}/api/v1/xml" |
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.
check the env variable OPENML_USE_LOCAL_SERVICES and update server accordingly
@pytest.fixture(autouse=True)
def with_server(request):
if os.getenv("OPENML_USE_LOCAL_SERVICES") == "true":
openml.config.TEST_SERVER_URL = "http://localhost:8080"
if "production" in request.keywords:
openml.config.server = "https://www.openml.org/api/v1/xml"
openml.config.apikey = None
yield
return
openml.config.server = f"{openml.config.TEST_SERVER_URL}/api/v1/xml"
openml.config.apikey = TestBase.user_key
yield
Metadata
Details
This PR implements the setting up of the v1 and v2 test servers in CI using docker via
localhost.