q-m / scrapyd-k8s

Scrapyd on container infrastructure
MIT License
12 stars 7 forks source link

Add proper error messages for unsupported endpoints (q-m#16) #29

Closed RuurdBijlsma-Ordina closed 6 months ago

RuurdBijlsma-Ordina commented 6 months ago

I used http status 501 (Not Implemented) for this, but if you think 404 is better, I'll change it.

I took the messages sent with the error from the README and added basic tests.

Fixes #16

RuurdBijlsma-Ordina commented 6 months ago

btw I wasn't 100% sure how to name the tests, or if they are even needed. I went with test_addversion, test_delversion, test_delproject

RuurdBijlsma-Ordina commented 6 months ago

Ah, super! Glad you were able to come up with a PR like this so fast, well done 🚀 One thing you could improve: mention the issue number in the PR body, e.g. "Fixes #16".

Ah yea will do for this one and coming ones, thanks.

wvengen commented 6 months ago

I'm doubting a bit between 404 and 501 as the http status. I think 501 fine, I'll accept it :)