Rework list_tasks: change raise error to warning for bad tasks to avoid crashing for bad server state by LennartPurucker · Pull Request #1244 · openml/openml-python · GitHub
Skip to content

Rework list_tasks: change raise error to warning for bad tasks to avoid crashing for bad server state #1244

Merged
mfeurer merged 1 commit into
developfrom
skip_bad_task_not_crash
Jun 12, 2023
Merged

Rework list_tasks: change raise error to warning for bad tasks to avoid crashing for bad server state #1244
mfeurer merged 1 commit into
developfrom
skip_bad_task_not_crash

Conversation

@LennartPurucker

Copy link
Copy Markdown
Contributor

Closes #1234

@codecov-commenter

Copy link
Copy Markdown

@mfeurer

mfeurer commented Apr 17, 2023

Copy link
Copy Markdown
Collaborator

Hey, is this still an issue? I don't think we should add workaround code for uncommon bugs on the server (i.e. they happened only once so far) and rather fix things on the server instead.

@LennartPurucker

Copy link
Copy Markdown
Contributor Author

Hey, is this still an issue? I don't think we should add workaround code for uncommon bugs on the server (i.e. they happened only once so far) and rather fix things on the server instead.

Mhm, I would say the current workflow (i.e., crashing) is an issue, and the alternative workflow (i.e., warning instead of crashing) would be better. But I see your point that we technically are only following the server specification.

I would leave this to your judgment. Feel free to close the PR.

@PGijsbers

PGijsbers commented Apr 25, 2023

Copy link
Copy Markdown
Collaborator

I vote +1 on merging this.

Philosophically I agree that openml-python should not accommodate to all kind of buggy server states. However specifically for this case, I think the pragmatic approach to issue a warning instead of an error leads to a much better user experience. My reason for preferring this is because the error is more likely than not caused by tasks in which the user had no interest to begin with (to contrast, I would not add work-arounds for actually downloading and instantiating such a corrupted task, for example).

@mfeurer

mfeurer commented Jun 12, 2023

Copy link
Copy Markdown
Collaborator

@mfeurer mfeurer merged commit a4ec4bc into develop Jun 12, 2023
@LennartPurucker LennartPurucker deleted the skip_bad_task_not_crash branch October 18, 2024 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

KeyError on list_tasks

4 participants