Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed requests contrib to raise original exceptions #771

Conversation

victoraugustolls
Copy link
Contributor

Updates requests contrib to raise original exceptions, and updated tests.

Thanks @isra17 for noticing!

@victoraugustolls
Copy link
Contributor Author

victoraugustolls commented Aug 22, 2019

With this and #755 I'm hopping we can release a new minor version?
@reyang @c24t

([#755](https://github.com/census-instrumentation/opencensus-python/pull/755))
- Added `http code` to `grpc code` status code mapping on `utils`
([#746](https://github.com/census-instrumentation/opencensus-python/pull/746))
- Updated `requests` module
([#771](https://github.com/census-instrumentation/opencensus-python/pull/771))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to update the CHANGELOG for the core sdk?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is just for tracking for the next release, so we don’t forget to update some module. I remember someone suggested this recently, but I can’t remember in which PR. It does not need to go into the release changelog (in my opinion), but this way we know what needs to be updated !

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. As long as we are consistent, I feel like it should be fine. So far it seems like we only update the changelog if there are changes in the actual package but your argument for it being a "release reminder" is valid :).

Copy link
Contributor

@reyang reyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Contributor

@songy23 songy23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@c24t is on vacation for a few days. Maybe @reyang can help with the release?

@reyang
Copy link
Contributor

reyang commented Aug 22, 2019

@songy23 Sure, we'll take care of it.

@reyang reyang merged commit a8c3415 into census-instrumentation:master Aug 22, 2019
@victoraugustolls victoraugustolls deleted the hotfix/requests-exception branch August 22, 2019 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants