Issue 1028: public delete functions for run, task, flow and database #1060
Conversation
PGijsbers
left a comment
PGijsbers
left a comment
There was a problem hiding this comment.
@mfeurer Should we want additional error wrapping?
E.g. deleting a task with runs says:
>>> openml.tasks.delete_task(4263)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "E:\repositories\openml-python\openml\tasks\functions.py", line 537, in delete_task
return openml.utils._delete_entity("task", task_id)
File "E:\repositories\openml-python\openml\utils.py", line 175, in _delete_entity
result_xml = openml._api_calls._perform_api_call(url_suffix, "delete")
File "E:\repositories\openml-python\openml\_api_calls.py", line 61, in _perform_api_call
response = __read_url(url, request_method, data)
File "E:\repositories\openml-python\openml\_api_calls.py", line 160, in __read_url
return _send_request(
File "E:\repositories\openml-python\openml\_api_calls.py", line 192, in _send_request
__check_response(response=response, url=url, file_elements=files)
File "E:\repositories\openml-python\openml\_api_calls.py", line 230, in __check_response
raise __parse_server_exception(response, url, file_elements=file_elements)
openml.exceptions.OpenMLServerException: https://www.openml.org/api/v1/xml/task/4263 returned code 454: Task is executed in some runs. Delete these first - None
Deleting something not owned by you:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "E:\repositories\openml-python\openml\runs\functions.py", line 1208, in delete_run
return openml.utils._delete_entity("run", run_id)
File "E:\repositories\openml-python\openml\utils.py", line 175, in _delete_entity
result_xml = openml._api_calls._perform_api_call(url_suffix, "delete")
File "E:\repositories\openml-python\openml\_api_calls.py", line 61, in _perform_api_call
response = __read_url(url, request_method, data)
File "E:\repositories\openml-python\openml\_api_calls.py", line 160, in __read_url
return _send_request(
File "E:\repositories\openml-python\openml\_api_calls.py", line 192, in _send_request
__check_response(response=response, url=url, file_elements=files)
File "E:\repositories\openml-python\openml\_api_calls.py", line 230, in __check_response
raise __parse_server_exception(response, url, file_elements=file_elements)
openml.exceptions.OpenMLServerException: https://www.openml.org/api/v1/xml/run/10559886 returned code 393: Run is not owned by you - None
The messages are pretty clear though the stack trace perhaps a bigger than expected by an end-user. We could also use e.g. PermissionError for trying to delete others' entities.
@Mirkazemi Please include tests for common errors (deleting something that is not yours, deleting an entity with attached other entities (e.g. task with runs)). I suggest you wait with this until Matthias has chimed in on whether or not the exceptions should be wrapped.
|
|
||
| run = openml.runs.run_model_on_task(model=clf, task=task, seed=rs) | ||
| run.publish() | ||
| TestBase._mark_entity_for_removal("run", run.run_id) |
There was a problem hiding this comment.
I suppose we want to mark the entities in other tests for removal as well - even though the tests should remove them. In case we introduce some error in the delete functions the entities will still be cleaned from the test server.
|
I agree, we could add two new exceptions for the cases you mentioned above to make it easier for the user to understand what's going on and automatically parse the exceptions. |
|
Hi @Mirkazemi! I hope you are well :) If you're planning to continue working on this PR please let us know. |
PGijsbers
force-pushed
the
issue_1028
branch
from
880a597 to
ebcd522
Compare
|
@mfeurer Can you have a quick look at the progress? In particular the testing against cached
With the new server implementation, the server tests themselves will be responsible for ensuring the right behaviour for provided input. However, this will take a while to roll out (and for openml-python to use), so there is also a pro to actually doing integration tests (the
|
mfeurer
left a comment
mfeurer
left a comment
There was a problem hiding this comment.
A few minor things, but besides that, this PR looks good to me.
|
Thanks! I'll process the feedback later and add similar tests for the other entities. I'll ping you when I'm done :) |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #1060 +/- ##
===========================================
+ Coverage 85.16% 85.24% +0.07%
===========================================
Files 38 38
Lines 4981 5008 +27
===========================================
+ Hits 4242 4269 +27
Misses 739 739
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
Thanks @Mirkazemi for starting this PR 🎉 |
4 participants
Reference Issue
Public delete methods #1028
What does this PR implement/fix? Explain your changes.
The new functions allow the user to delete run, task, flow and database using API:
openml.runs.delete_run()
openml.tasks.delete_task()
openml.flows.delete_flow()
openml.datasets.delete_dataset()
How should this PR be tested?
The unit tests are added to the
tests/test_runs/test_run_functions.py
tests/test_tasks/test_task_functions.py
tests/test_flows/test_flow_functions.py
tests/test_datasets/test_dataset_functions.py