Raising an error when response returns any status code but 200. #11
Conversation
| @@ -223,7 +226,9 @@ def test_jobs(self, python_version, repo_slug): | |||
| assert build == job.build | |||
|
|
|||
| job.build_id = -1 | |||
| assert job.build is None | |||
| with pytest.raises(TravisError) as exception_info: | |||
| job.build | |||
There was a problem hiding this comment.
While I really like TravisPy.github_auth raising an error, raising an exception for job.build and job.log feels weird for me. Seems that it may lead to code like this:
try:
build = job.build
except TravisError:
# not sure what the user can do in this case
else:
# job has a build Can you give an example in which real-life situation a job wouldn't have a associated build or log? I haven't used the travis api extensively, so some enlightment here is appreciated! 😄
There was a problem hiding this comment.
Both properties build and log uses related *_id to actually load/create objects. This is basically to test how _load_lazy_information will behave when facing invalid IDs. I am just foreseeing this kind of problem, it never happened so far. Maybe it is a gold plate that is worth to remove.
There was a problem hiding this comment.
I see, so this is an exceptional case and not really part a common usage scenario.
Then I guess this is just fine the way it is, thanks!
|
Looks good to me, feel free to merge! |
Raising an error when response returns any status code but 200.
This fixes #10