Integrate rollbar exception handling as an optional feature#346
Closed
oshoma wants to merge 13 commits intoAggregate-Intellect:mainfrom
Closed
Integrate rollbar exception handling as an optional feature#346oshoma wants to merge 13 commits intoAggregate-Intellect:mainfrom
oshoma wants to merge 13 commits intoAggregate-Intellect:mainfrom
Conversation
oshoma
force-pushed
the
integrate-rollbar
branch
from
38f315e to
7fa6af9
Compare
oshoma
force-pushed
the
integrate-rollbar
branch
4 times, most recently
from
54126e3 to
de6a507
Compare
Run `poetry self update && poetry install` after picking up this commit. Changes: - Change all make steps to depend on poetry being installed - Add mypy for static type checking, but leave disabled for now - Use more sophisticated lint options - Include Slack app in `make test` - Include Slack tests in `make lint` - Fail tests if coverage drops below 75% mypy gives us static type checking. It triggers many warnings, so I am disabling mypy for now and will create an issue to resolve mypy warnings.
Change code to resolve these lint warnings: - F821: Undefined names - F841: Local variable name is assigned to but never used - F811: Redefinition of unused name from line name
Change code to resolve these warnings: - F811: Redefinition of unused name from line name - W291: Trailing whitespace `make lint` will now warn and fail if files contain trailing whitespace.
flake8 is currently configured to ignore these errors: W503: Line break occurred before a binary operator E501: Line too long (82 > 79 characters) E402: Module level import not at top of file This change makes the behavior consistent for both `make lint` and `poetry run flake8`. Also removing E402 as that warning was already resolved.
Collaborator
Author
Create a Makefile just for Slackapp so we can run `make test` within /sherpa/src/apps/slackapp using Slackapp's own Poetry environment. Remove knowledge of Slackapp from Sherpa's makefile. Change make to install Spacy model dependency.
Run `poetry self update && poetry install` to pick this up.
Fixes the following Bandit errors seen with `make lint`: 1. Issue: [B104:hardcoded_bind_all_interfaces] Possible binding to all interfaces. Severity: Medium Confidence: Medium CWE: CWE-605 (https://cwe.mitre.org/data/definitions/605.html) More Info: https://bandit.readthedocs.io/en/1.7.8/plugins/b104_hardcoded_bind_all_interfaces.html Location: apps/slackapp/slackapp/bolt_app.py:346:13 Change: suppress the warning with #nosec B104, since our use of host '0.0.0.0' is intentional. Also add a security note. 2. Issue: [B113:request_without_timeout] Requests call without timeout Location: apps/slackapp/slackapp/bolt_app.py:340:23 Location: sherpa_ai/scrape/extract_github_readme.py:75:19 Location: sherpa_ai/utils.py:83:15 Location: sherpa_ai/scrape/extract_github_readme.py:52:19 Location: sherpa_ai/scrape/file_scraper.py:69:19 Change: add timeouts for each of these. Adjustment may be needed. 3. Issue: [B310:blacklist] Audit url open for permitted schemes. Allowing use of file:/ or custom schemes is often unexpected. Severity: Medium Confidence: High CWE: CWE-22 (https://cwe.mitre.org/data/definitions/22.html) More Info: https://bandit.readthedocs.io/en/1.7.8/blacklists/blacklist_calls.html#b310-urllib-urlopen Location: sherpa_ai/tools.py:59:15 Location: sherpa_ai/utils.py:294:12 Change: add logic to ensure URLs are HTTP or HTTPS. Add tests.
poetry run flake8 --ignore=W503,E501,F401 sherpa_ai tests sherpa_ai/utils.py:497:8: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()` sherpa_ai/utils.py:507:13: F841 local variable 'e' is assigned to but never used sherpa_ai/utils.py:554:114: W291 trailing whitespace tests/integration_tests/test_entity_citation_validator.py:53:5: F811 redefinition of unused 'get_llm' from line 13 tests/integration_tests/test_qa_agent_actions.py:46:49: F811 redefinition of unused 'get_llm' from line 8 tests/integration_tests/test_qa_agent_actions.py:72:55: F811 redefinition of unused 'get_llm' from line 8
Reorganize environment variables in sherpa_ai/config and make matching changes in `.env-sample`. Also improve the documentation a wee bit.
This feature is optional. By default, Rollbar is not installed. To use Rollbar, follow these steps: ``` cd src/apps/slackapp poetry install -E rollbar ``` Then in your `.env` file: - add your ROLLBAR_ACCESS_TOKEN - set SHERPA_ENV to "development" Restart the Slack app. Sherpa will now forward exceptions to Rollbar, provided you are running in the development environment. When Rollbar is installed, `make test` runs additional tests. These tests are skipped when Rollbar is not installed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Integrate Rollbar exception handling into Slack app
Rollbar is an error tracking and monitoring service that catches exceptions and saves them so developers can inspect them later. It also supports automatic notifications to email, Slack, etc.
This feature is optional. By default, it is not installed as part of the Sherpa Slackapp.
Rollbar is most useful in a production or staging environment. That said, you can install
and use it locally if you want to try it out. To do so, follow these steps:
Then add to your
.envfile:And restart the Slack app:
poetry run python -m slackapp.bolt_appSherpa will now forward exceptions to Rollbar. You can test this by running
curl localhost:3000/test_debugagainst your running Sherpa Slackapp.When Rollbar is installed,
make testruns additional tests. These tests are skipped when Rollbar is not installed.Type of change
Related Issues
Depends on #343 . Please review and merge that first.
Checklists
To speed up the review process, please follow these checklists:
Development
make format && make lint)make test)See the testing guidelines for help on tests, especially those involving web services.
Code review
💔 Thank you for submitting a pull request!