Skip to content

poor man's patch for issue #630 #633

Merged
merged 1 commit into from May 29, 2012

5 participants

@tzuryby
tzuryby commented May 24, 2012
this try catch is a poor man's patch for issue #630
https://github.com/kennethreitz/requests/issues/630
@travisbot

This pull request passes (merged 7125fe5 into de1637c).

@kennethreitz
Owner

Thanks!

@kennethreitz kennethreitz merged commit 7813bd8 into kennethreitz:develop May 29, 2012
@kennethreitz kennethreitz added a commit that referenced this pull request May 29, 2012
@kennethreitz cleanup #633 1c0abbd
@mwielgoszewski

Hey Kenneth, I would recommend against wrapping the whole method within a try/except ValueError block. I think my pull request #641 should be sufficient in addressing @tzuryby's issue. If there is another edge case that would raise an exception here, it would be valuable for us to understand why it did so. Catching the exception and continuing would prevent us or the developer from even knowing there was an issue with the URI.

@piotr-dobrogost

I would recommend against wrapping the whole method within a try/except ValueError block

Agree.

@tzuryby
tzuryby commented May 31, 2012

that's a far better fix than my catch-all approach, no doubt.
that's why I called it "poor man's patch.." ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.