Skip to content

Commit f9530f1

Browse files
committed
implement the suggestions
1 parent d0d8f17 commit f9530f1

4 files changed

Lines changed: 30 additions & 32 deletions

File tree

openml/_api/resources/flow.py

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
from __future__ import annotations
22

3-
from collections.abc import Mapping
43
from typing import Any
4+
from urllib.parse import quote
55

66
import pandas as pd
77
import xmltodict
88

9-
from openml.exceptions import OpenMLServerError, OpenMLServerException
9+
from openml.exceptions import OpenMLServerException, OpenMLServerNoResult
1010
from openml.flows.flow import OpenMLFlow
1111

1212
from .base import FlowAPI, ResourceV1API, ResourceV2API
@@ -61,8 +61,11 @@ def exists(self, name: str, external_version: str) -> int | bool:
6161
if not (isinstance(external_version, str) and len(external_version) > 0):
6262
raise ValueError("Argument 'version' should be a non-empty string")
6363

64-
data = {"name": name, "external_version": external_version, "api_key": self._http.api_key}
65-
xml_response = self._http.post("flow/exists", data=data).text
64+
data: dict[str, str] = {"name": name, "external_version": external_version}
65+
if self._http.api_key:
66+
data["api_key"] = self._http.api_key
67+
68+
xml_response = self._http.post("flow/exists", data=data, use_api_key=False).text
6669
result_dict = xmltodict.parse(xml_response)
6770
# Detect error payloads and raise
6871
if "oml:error" in result_dict:
@@ -196,14 +199,20 @@ def exists(self, name: str, external_version: str) -> int | bool:
196199
if not (isinstance(external_version, str) and len(external_version) > 0):
197200
raise ValueError("Argument 'version' should be a non-empty string")
198201

202+
name_path = quote(name, safe="")
203+
version_path = quote(external_version, safe="")
204+
199205
try:
200-
response = self._http.get(f"flows/exists/{name}/{external_version}/")
206+
response = self._http.get(f"flows/exists/{name_path}/{version_path}/")
201207
result = response.json()
202208
flow_id: int | bool = result.get("flow_id", False)
203209
return flow_id
204-
except (OpenMLServerError, KeyError):
205-
# v2 returns 404 when flow doesn't exist, which raises OpenMLServerError
210+
except OpenMLServerNoResult:
206211
return False
212+
except OpenMLServerException as err:
213+
if err.code == 404:
214+
return False
215+
raise
207216

208217
def list(
209218
self,
@@ -214,9 +223,6 @@ def list(
214223
) -> pd.DataFrame:
215224
self._not_supported(method="list")
216225

217-
def publish(self, path: str | None = None, files: Mapping[str, Any] | None = None) -> int: # type: ignore[override] # noqa: ARG002
218-
self._not_supported(method="publish")
219-
220226
@staticmethod
221227
def _convert_v2_to_v1_format(v2_json: dict[str, Any]) -> dict[str, dict]:
222228
"""Convert v2 JSON response to v1 XML-dict format for OpenMLFlow._from_dict().

openml/flows/functions.py

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,17 @@
22
from __future__ import annotations
33

44
from functools import partial
5-
from typing import Any, cast
5+
from typing import Any
66

77
import dateutil.parser
88
import pandas as pd
99
import xmltodict
1010

1111
import openml
12-
import openml._api_calls
1312
import openml.utils
1413

1514
from . import OpenMLFlow
1615

17-
FLOWS_CACHE_DIR_NAME = "flows"
18-
1916

2017
@openml.utils.thread_safe_if_oslo_installed
2118
def get_flow(
@@ -182,9 +179,7 @@ def flow_exists(name: str, external_version: str) -> int | bool:
182179
if not (isinstance(external_version, str) and len(external_version) > 0):
183180
raise ValueError("Argument 'version' should be a non-empty string")
184181

185-
return cast(
186-
"int | bool", openml._backend.flow.exists(name=name, external_version=external_version)
187-
)
182+
return openml._backend.flow.exists(name=name, external_version=external_version)
188183

189184

190185
def get_flow_id(
@@ -511,4 +506,4 @@ def delete_flow(flow_id: int) -> bool:
511506
>>> # Deletes flow 23 if you are the uploader and it's not linked to runs
512507
>>> openml.flows.delete_flow(23) # doctest: +SKIP
513508
"""
514-
return openml._backend.flow.delete(flow_id) # type: ignore
509+
return openml._backend.flow.delete(flow_id)

tests/test_api/test_flow.py

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ def test_flow_v1_exists_input_validation(flow_v1):
7474
flow_v1.exists(name="sklearn.tree.DecisionTreeClassifier", external_version="")
7575

7676

77-
def test_flow_v1_exists_mocked_success(flow_v1, test_apikey_v1):
77+
def test_flow_v1_exists_mocked_success(flow_v1):
7878
flow_name = "sklearn.tree.DecisionTreeClassifier"
7979
external_version = "1"
8080

@@ -90,18 +90,15 @@ def test_flow_v1_exists_mocked_success(flow_v1, test_apikey_v1):
9090
result = flow_v1.exists(name=flow_name, external_version=external_version)
9191

9292
assert result == 123
93-
mock_request.assert_called_once_with(
94-
method="POST",
95-
url=openml.config.server + "flow/exists",
96-
params={},
97-
data={
98-
"name": flow_name,
99-
"external_version": external_version,
100-
"api_key": test_apikey_v1,
101-
},
102-
headers=openml.config._HEADERS,
103-
files=None,
104-
)
93+
mock_request.assert_called_once()
94+
_, kwargs = mock_request.call_args
95+
assert kwargs["method"] == "POST"
96+
assert kwargs["url"] == openml.config.server + "flow/exists"
97+
assert kwargs["params"] == {}
98+
assert kwargs["headers"] == openml.config._HEADERS
99+
assert kwargs["files"] is None
100+
assert kwargs["data"]["name"] == flow_name
101+
assert kwargs["data"]["external_version"] == external_version
105102

106103

107104
def test_flow_v1_exists_mocked_server_error(flow_v1):

tests/test_flows/test_flow.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ def test_publish_error(self, mock_request, flow_exists_mock, get_flow_mock):
329329
"<oml:upload_flow>\n" " <oml:id>1</oml:id>\n" "</oml:upload_flow>"
330330
).encode()
331331
mock_request.return_value = response
332-
flow_exists_mock.return_value = None # Flow doesn't exist yet, so try to publish
332+
flow_exists_mock.return_value = False # Flow doesn't exist yet, so try to publish
333333
get_flow_mock.return_value = flow
334334

335335
flow.publish()

0 commit comments

Comments
 (0)