[ENH] V1 -> V2 Migration : Runs by Omswastik-11 · Pull Request #1616 · openml/openml-python

Omswastik-11 · 2026-01-15T09:09:13Z

Metadata

Reference Issue:
New Tests Added:
Documentation Updated:
Change Log Entry:

Details

codecov-commenter · 2026-01-15T20:58:38Z

Codecov Report

❌ Patch coverage is 72.35772% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.89%. Comparing base (1f6fed4) to head (8c8426a).

Files with missing lines	Patch %	Lines
openml/_api/resources/run.py	75.94%	19 Missing ⚠️
openml/_api/clients/http.py	59.09%	9 Missing ⚠️
openml/runs/run.py	68.42%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1616      +/-   ##
==========================================
+ Coverage   81.45%   81.89%   +0.43%     
==========================================
  Files          63       63              
  Lines        5124     5170      +46     
==========================================
+ Hits         4174     4234      +60     
+ Misses        950      936      -14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

geetu040

sync with base pr
sdk code look good so far, please take a look at #1575 (comment) and make changes accordingly where needed.
all tests (existing and new) should pass to make sure we are retaining the original functionality of the sdk

Signed-off-by: Omswastik-11 <omswastikpanda11@gmail.com>

…into runs-migration-stacked

Signed-off-by: Omswastik-11 <omswastikpanda11@gmail.com>

…-11/openml-python into runs-migration-stacked

Signed-off-by: Omswastik-11 <omswastikpanda11@gmail.com>

geetu040 · 2026-01-30T14:10:51Z

+    use_cache = not ignore_cache
+    reset_cache = ignore_cache
+    return api_context.backend.runs.get(
+        run_id,
+        use_cache=use_cache,
+        reset_cache=reset_cache,
+    )


use_cache should be true since the method always supports caching
reset_cache should rely on ignore_cache

for more information, see https://pre-commit.ci

Copilot

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.

geetu040

Nicely done @Omswastik-11.

@PGijsbers, could you please review/merge this PR when you get a chance?

There is currently one issue caused by differences between the test-server and local-server database entities, which is temporarily patched here: #1616 (comment).

I had mentioned this earlier on Slack as well here, we can continue discussion there

PGijsbers

small changes or clarifications requested, please see comments.

PGijsbers · 2026-05-20T12:43:58Z

+        path_parts = parsed_url.path.strip("/").split("/")
+
+        filtered_params = {k: v for k, v in params.items() if k != "api_key"}
+        params_part = [urlencode(filtered_params)] if filtered_params else []


This is a good remark, but seeing as this code isn't touched by this PR, I would advocate fixing this in a separate PR.

PGijsbers · 2026-05-20T12:51:43Z

+        if response.content.startswith(b"PK\x03\x04"):
+            return "body.zip"
+
+        try:
+            arff.loads(response.text)
+            return "body.arff"
+        except arff.ArffException:
+            pass


Is there no HTTP header data that would allow us to tell what the content (and file name) should be?
Otherwise, at least for ARFF, the spec states that the first non-comment line of the file should be (not case sensitive): @relation <relation name>. So we could look for that instead of parsing the entire file content.

Is there no HTTP header data that would allow us to tell what the content (and file name) should be?

I tried but didn't find anything

Otherwise, at least for ARFF, the spec states that the first non-comment line of the file should be (not case sensitive): @relation <relation name>.

sounds good I could give this a try

fixed in 5c20f22

PGijsbers · 2026-05-20T13:06:07Z

+        OpenMLHashException
+            If checksum verification fails.
+        """
+        url = urljoin(self.server, path)


ignore: If this isn't the case already, this should be normalized when openml.config.server is set, not each site which uses it.

PGijsbers · 2026-05-20T13:10:55Z

+        if use_api_key:
+            params["api_key"] = self.api_key
+
+        if method.upper() in {"POST", "PUT", "PATCH"}:
+            data = {**params, **data}


ignore: It raises an exception if api_key is None, it's the statement preceding this line..

PGijsbers · 2026-05-20T13:12:28Z

+        self,
+        limit: int,
+        offset: int,
+        *,
+        ids: builtins.list[int] | None = None,


please address or explain; i see you have dismissed previous comments about this so presumably there is a reason?

PGijsbers · 2026-05-20T13:15:37Z

+
+        # Fall back to generic oml:id (used by other resources)
+        if "oml:id" in root_value:
+            return int(root_value["oml:id"])


If run responses always return oml:run_id, when do we expect this code path to be correct to run?

@Omswastik-11 since this method is overriden for runs, we shouldn't expect to handle other resources here, therefore logically this path should be unreachable as Pieter has said

Yeah got it. I removed it.

PGijsbers · 2026-05-20T13:25:15Z

+def test_run_v1_get(run_v1, with_test_cache):
+    try:
+        run = run_v1.get(run_id=1)
+    except OpenMLServerException as e:
+        if e.code == 236 or "Run not found" in str(e):
+            run = run_v1.get(run_id=25)
+        else:
+            raise
+    _assert_run_shape(run)
+


Didn't we have a way to check whether a local or non-local server configured is being used?
Then I would prefer to use that e.g.,

run_id = 25 if openml config is local else 1

That embeds this knowledge into the code so it's clear for future maintainers.
We probably do not have the time to address this on our end for a while longer :(

geetu040 · 2026-05-21T19:34:56Z

@Omswastik-11 could you go through the above comments, we'd need to close these discussions.

Copilot

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

openml/runs/run.py:373

The ObjectNotPublishedError message here diverges from the established tagging error message used by OpenMLBase.remove_tag (via openml.utils._tag_openml_base), and it also drops the object context. Consider reusing the same wording/format for consistency across entity types.

        if self.run_id is None:
            raise openml.exceptions.ObjectNotPublishedError(
                "Cannot untag a run that has not been published yet."
                " Please publish the run first before being able to untag it.",
            )

Co-authored-by: Pieter Gijsbers <p.gijsbers@tue.nl>

Copilot

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.

…-11/openml-python into runs-migration-stacked

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

geetu040 · 2026-05-23T09:20:07Z

+        if len(candidates) > 1:
+            raise FileNotFoundError(
+                f"Multiple body files found in path: {path} ({[p.name for p in candidates]})"
+            )
+
+        return candidates[0].name



I don't think this case can be expected given the v1/v2 endpoints

geetu040 · 2026-05-23T09:21:40Z

+def test_run_v1_get(run_v1, with_test_cache):
+    import os
+
+    # Run 1 exists on the remote test server; the local docker server only seeds run 25.
+    run_id = 25 if os.getenv("OPENML_USE_LOCAL_SERVICES") == "true" else 1
+    run = run_v1.get(run_id=run_id)
+    _assert_run_shape(run)


we'll use cached run instead, so this can be ignored

geetu040 · 2026-05-23T09:22:35Z

+    # Run 1 exists on the remote test server; the local docker server only seeds run 25.
+    run_id = 25 if os.getenv("OPENML_USE_LOCAL_SERVICES") == "true" else 1
+    run = run_v1.get(run_id=run_id)
+    _assert_run_shape(run)


I believe you were trying to do this in the start. this would be the right way to use run from cache to avoid different entities on both servers.

- def test_run_v1_get(run_v1, with_test_cache): - import os - - # Run 1 exists on the remote test server; the local docker server only seeds run 25. - run_id = 25 if os.getenv("OPENML_USE_LOCAL_SERVICES") == "true" else 1 - run = run_v1.get(run_id=run_id) - _assert_run_shape(run) + def test_run_v1_get(run_v1, test_files_directory): + openml.config.set_root_cache_directory(test_files_directory) + run = run_v1.get(run_id=1) + _assert_run_shape(run)

updated in 7e57779

Copilot

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

geetu040 mentioned this pull request Jan 15, 2026

[ENH] V1 → V2 API Migration #1575

Open

18 tasks

geetu040 assigned Omswastik-11 Jan 19, 2026

geetu040 suggested changes Jan 30, 2026

View reviewed changes

Comment thread openml/_api/resources/runs.py Outdated

Comment thread openml/_api/resources/runs.py Outdated

Comment thread openml/_api/resources/runs.py Outdated

Comment thread openml/runs/functions.py Outdated

Omswastik-11 added 3 commits January 30, 2026 14:56

tests:added tests for migration

a70a33f

Signed-off-by: Omswastik-11 <omswastikpanda11@gmail.com>

Merge branch 'migration' of https://github.com/geetu040/openml-python …

d08b1fe

…into runs-migration-stacked

tests:added tests for migration

aaf9d4b

Signed-off-by: Omswastik-11 <omswastikpanda11@gmail.com>

Omswastik-11 requested a review from geetu040 January 30, 2026 09:50

Merge branch 'main' into runs-migration-stacked

84d43a9

Omswastik-11 marked this pull request as ready for review January 30, 2026 09:50

Omswastik-11 added 6 commits January 30, 2026 18:29

tests:modified old chache tests to use new caching

761ef9e

Signed-off-by: Omswastik-11 <omswastikpanda11@gmail.com>

Merge branch 'runs-migration-stacked' of https://github.com/Omswastik…

cf37a75

…-11/openml-python into runs-migration-stacked

tests:modified old chache tests to use new caching

2eb7c7a

Signed-off-by: Omswastik-11 <omswastikpanda11@gmail.com>

tests:modified old chache tests to use new caching

d9476c8

Signed-off-by: Omswastik-11 <omswastikpanda11@gmail.com>

tests:skip the production related tests

f4718c1

Signed-off-by: Omswastik-11 <omswastikpanda11@gmail.com>

tests:modify skip messgaes

4560b6b

Signed-off-by: Omswastik-11 <omswastikpanda11@gmail.com>

geetu040 suggested changes Jan 30, 2026

View reviewed changes

SimonBlanke and others added 13 commits January 30, 2026 19:27

add 'get_api_config' skeleton method

1913c10

remove 'APISettings'

7681949

impl. 'get_api_config'

01840a5

add singleton pattern for settings

26ed4c1

add 'reset_settings'

c588d0c

remove unused code

b6ff720

reimplement usage of v1 settings config

80d5afc

first try v2, fallback to v1 if not available

f47112c

reimplement singelton without the use of 'global'

d44cf3e

add explanations

ea7dda1

change usage of settings to new impl.

f0e5947

add explanations

edcd006

[pre-commit.ci] auto fixes from pre-commit.com hooks

cde0aae

for more information, see https://pre-commit.ci

Merge branch 'main' into runs-migration-stacked

65ee864

Copilot AI review requested due to automatic review settings May 11, 2026 12:35

Copilot started reviewing on behalf of Omswastik-11 May 11, 2026 12:35 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

Comment thread openml/_api/clients/http.py

Comment thread openml/_api/clients/http.py Outdated

Comment thread openml/_api/resources/run.py Outdated

Comment thread openml/_api/resources/base/resources.py

Comment thread tests/test_api/test_run.py Outdated

only overrid _extract_id_from_upload

646ce1b

Omswastik-11 requested a review from geetu040 May 11, 2026 12:47

geetu040 approved these changes May 11, 2026

View reviewed changes

PGijsbers requested changes May 20, 2026

View reviewed changes

Merge branch 'main' into runs-migration-stacked

ff1d6bd

Copilot AI review requested due to automatic review settings May 22, 2026 12:58

Copilot started reviewing on behalf of Omswastik-11 May 22, 2026 12:59 View session

Copilot AI reviewed May 22, 2026

View reviewed changes

Comment thread openml/_api/resources/base/resources.py

Comment thread tests/test_api/test_run.py Outdated

Comment thread openml/runs/run.py

Comment thread openml/_api/clients/http.py Outdated

Update openml/_api/clients/http.py

9ce181f

Co-authored-by: Pieter Gijsbers <p.gijsbers@tue.nl>

Copilot AI review requested due to automatic review settings May 22, 2026 13:06

Merge branch 'main' into runs-migration-stacked

16c5fcc

Copilot started reviewing on behalf of Omswastik-11 May 22, 2026 13:07 View session

Copilot AI reviewed May 22, 2026

View reviewed changes

Comment thread openml/_api/resources/base/resources.py

Comment thread openml/_api/resources/run.py Outdated

Comment thread openml/_api/clients/http.py Outdated

Comment thread openml/_api/clients/http.py

Comment thread tests/test_api/test_run.py Outdated

Omswastik-11 and others added 3 commits May 22, 2026 18:49

fix: address PR review comments on runs migration

a5eae2c

Merge branch 'runs-migration-stacked' of https://github.com/Omswastik…

f340a27

…-11/openml-python into runs-migration-stacked

Apply suggestion from @Copilot

f2363ed

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings May 22, 2026 13:32

Copilot started reviewing on behalf of Omswastik-11 May 22, 2026 13:32 View session

Omswastik-11 requested a review from PGijsbers May 22, 2026 13:33

Copilot AI reviewed May 22, 2026

View reviewed changes

geetu040 suggested changes May 23, 2026

View reviewed changes

geetu040 added 2 commits May 23, 2026 11:36

improve .arff check

5c20f22

update test_run_v1_get

7e57779

Copilot AI reviewed May 23, 2026

View reviewed changes

rerun ci

8c8426a

Uh oh!

Conversation

Omswastik-11 commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Metadata

Details

Uh oh!

codecov-commenter commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

geetu040 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

geetu040 left a comment

Choose a reason for hiding this comment

Uh oh!

PGijsbers left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geetu040 commented May 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Omswastik-11 commented Jan 15, 2026 •

edited

Loading

codecov-commenter commented Jan 15, 2026 •

edited

Loading