[ENH] V1 → V2 API Migration - core structure#1576
[ENH] V1 → V2 API Migration - core structure#1576geetu040 wants to merge 219 commits intoopenml:mainfrom
Conversation
…into issue1564
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1576 +/- ##
==========================================
- Coverage 52.73% 52.37% -0.37%
==========================================
Files 37 61 +24
Lines 4399 5035 +636
==========================================
+ Hits 2320 2637 +317
- Misses 2079 2398 +319 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
openml#1576 (comment) Co-authored-by: Pieter Gijsbers <p.gijsbers@tue.nl>
openml#1576 (comment) Co-authored-by: Pieter Gijsbers <p.gijsbers@tue.nl>
openml#1576 (comment) Co-authored-by: Pieter Gijsbers <p.gijsbers@tue.nl>
for more information, see https://pre-commit.ci
This reverts commit 93155ee.
|
FYI: @JATAYU000 @satvshr @omkar-334 @rohansen856 @EmanAbdelhaleem Please sync with this PR. The CI tests should now be passing in all the stacked PRs (except some sporadic failures), since now they are using the local docker services created in the CI which provides endpoints for v2-api as well. You can run these tests locally as well, with the following steps: Run docker services locally: git clone --depth 1 https://github.com/openml/services.git
cd ./services
chmod -R a+rw ./data
chmod -R a+rw ./logs
docker compose --profile rest-api --profile minio --profile evaluation-engine up -dRun pytest using the local services (add the env var): OPENML_USE_LOCAL_SERVICES="true" pytest tests/ |
| _TEST_SERVERS_LOCAL: dict[APIVersion, dict[str, str | None]] = { | ||
| APIVersion.V1: { | ||
| "server": "http://localhost:8000/api/v1/xml/", | ||
| "apikey": "normaluser", | ||
| }, | ||
| APIVersion.V2: { | ||
| "server": "http://localhost:8082/", | ||
| "apikey": "AD000000000000000000000000000000", | ||
| }, | ||
| } |
There was a problem hiding this comment.
Presumably they use the same database, is there a reason for using different API keys?
| _SERVERS_REGISTRY: dict[str, dict[APIVersion, dict[str, str | None]]] = { | ||
| "production": _PROD_SERVERS, | ||
| "test": _TEST_SERVERS_LOCAL | ||
| if os.getenv("OPENML_USE_LOCAL_SERVICES") == "true" | ||
| else _TEST_SERVERS, | ||
| } | ||
|
|
||
|
|
||
| def _get_servers(mode: str) -> dict[APIVersion, dict[str, str | None]]: |
There was a problem hiding this comment.
I think it would probably be best to have possible servers in a StrEnum so that the type hints can communicate expected valid values. It's true that users could technically extend the _SERVERS_REGISTERY at runtime and thus more values would be valid but:
- that's probably incredibly uncommon
- uses 'internal' functionality (there is a
_prefix) - the user can still do that, it's just that the type hint wouldn't be accurate if they do
I considered instead type hints with string literals, but considering the amount of call sites below, I think an enum makes more sense.
Towards #1575
This PR sets up the core folder and file structure along with base scaffolding for the API v1 → v2 migration.
It includes:
*V1,*V2)No functional endpoints are migrated yet. This PR establishes a stable foundation for subsequent migration and refactor work.