Unifies NotFound error handling #126
Conversation
|
Did not mean to close. Anything more I should do for this? |
|
@rthbound Hi Tad, sorry about the silence -- I have the PR checked out locally, but had to switch priorities. This is a significant change (LOC-wise) so I wanted to think about it a bit. Please ping me in couple of days if I won't reply or merge it. |
|
Closing & Reopening to re-trigger travis build (and to ping @karmi ) I think these build failures are intermittent. |
|
@rthbound Thanks for staying on this! I've been buried in the |
- Util#__rescue_from_not_found method to standardize NotFound handling
- Unit tests for new method
Example:
Utils.__rescue_from_not_found do
# ...
perform_request(method, path, params, body).status == 200 ? true : false
end
rthbound
force-pushed
the
unifies_not_found_exception_handling
branch
from
22374c4
to
9635384
Compare
|
@karmi No problem! Thanks for the git guidance, and sorry to generate so much noise with this one. |
|
@rthbound Nothing to worry about!, it's not a problem at all! It was just really an advice :) |
|
@rthbound Sorry for sitting on this for so long! I'm glad you did it -- when looking at the code, I just think I would rearrange the code a bit and put the |
|
Sorry for the delay, I've been away on my honeymoon. That sounds excellent,
thanks!
…-Tad Hosford
|
|
@rthbound Didn't forget, have it checked out locally, but priorities keep popping up before this one... Sorry about that, it's not dead! |
|
@rthbound Finally got to merge this! Many thanks for the patch and sorry it took so long! |
|
@karmi No problem, glad to contribute! |
|
@karmi I've recently learned that we may be taking a performance hit by capturing the block object... How would you feel about removing the |
I noticed a bit of duplication in the 404/NotFound exception handling across several of the classes under Elasticsearch::API. This pull request should clear that up.