Rework list_tasks: change raise error to warning for bad tasks to avoid crashing for bad server state #1244
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## develop #1244 +/- ##
===========================================
- Coverage 85.24% 85.12% -0.12%
===========================================
Files 38 38
Lines 5008 5009 +1
===========================================
- Hits 4269 4264 -5
- Misses 739 745 +6
... and 1 file with indirect coverage changes 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. |
|
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. |
|
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). |
|
Merging this now as two people would like to have this merged. |
Closes #1234