rollbar
Do not force the requests module to always be included
Description of the change
Allows the module to work if requests is not available.
Type of change
- [ ] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Maintenance
- [x] New release
Related issues
Shortcut stories and GitHub issues (delete irrelevant)
- Fix [SC-]
- Fix #417
Checklists
Development
- [ ] Lint rules pass locally
- [ ] The code changed/added as part of this pull request has been covered with tests
- [ ] All tests related to the changed code pass in development
Code review
- [ ] This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
- [ ] "Ready for review" label attached to the PR and reviewers assigned
- [ ] Issue from task tracker has a link to this pull request
- [ ] Changes have been reviewed by at least one other engineer
I really just wanted to get this out quickly to see what CI would say, I'm not very confidant in my changes there. I was also surprised that all of the tests passed locally with no other changes but maybe that's a good thing?
I tried to make as small and as targeted of a change as possible but it's tricky, there is a lot of "hand shake" type of agreements. I don't have a ton of confidence that a user wouldn't get into a code path that refers to the requests module.
But I suppose if all the tests pass in the FastAPI/Starlette environments that doesn't have requests installed then maybe we're in the clear.
@danielmorell could I get some feedback on my solution here? Is this in the right direction?
Hey @phillipuniverse, sorry for the long wait! Overall, I think this is moving in the right direction, and solving a very real problem. Thank you for your work on this!
It may make sense to check if requests is available when Rollbar is initialized instead of waiting until we try to process the first error message. I feel like letting the developer know as soon as possible that they have configured a state that won't work would be best.
What do you think?
