get_article shouldn't raise RuntimeError

openzim / python-libzim

Libzim binding for Python: read/write ZIM files in Python

https://pypi.org/project/libzim/

GNU General Public License v3.0

62 stars 20 forks source link

get_article shouldn't raise RuntimeError #38

Closed rgaudin closed 4 years ago

rgaudin commented 4 years ago

Currently, ReadArticle.get_article() raises a RuntimeError should the URL be incorrect. I believe we should either:

Add an .has_article(url) methods that returns a boolean informing about the existence of the article
Raise a custom, usable Exception like ArticleNotFound or something.

mgautierfr commented 4 years ago

Yes, RuntimeError is a base, generic exception. We could raise a more specific one. However, with the planned change in the api openzim/libzim#345, this will change. We should wait for the new api to decide what to do.

rgaudin commented 4 years ago

Reading cython doc and this SO answer (and followings), it seems that we'd still have to declare and map the actual exception as the default behavior is to map everything (but a few standard ones) to RuntimeError.

So should we raise a custom (python for now) NotFound(RuntimeError) exception until https://github.com/openzim/libzim/issues/345 gets implemented and we map the actual NotFound?

mgautierfr commented 4 years ago

Indeed, we will need a custom exception handler both with the new api and the current one.

But I'm not sure that we should spend time on this now as it will change in a close future.