This repository has been archived by the owner on Jun 5, 2023. It is now read-only.
Add some exception handling around the config files. #6
Conversation
This file contains 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
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
1 similar comment
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
|
LGTM |
carise
approved these changes
Feb 22, 2017
| """ | ||
| try: | ||
| with open(CONFIGS_FILE, 'r') as config_file: | ||
| try: |
There was a problem hiding this comment.
I don't see you doing anything novel with the inner try, do we need to catch it's exception any differently?
| try: | ||
| with open(configs_path, 'r') as config_file: | ||
| try: | ||
| configs = yaml.load(config_file) |
arbeit
added a commit
to arbeit/forseti-security
that referenced
this pull request
Mar 27, 2018
blueandgold
pushed a commit
that referenced
this pull request
Mar 27, 2018
…1290) * Add test to show that no empty notifications are sent * work in progress * fixed test * better test data * Use test data from the fake_violations module * address review comments, fix up test data * addressing review comments, batch #2 * addressing review comments, batch #3 * addressing review comments, batch #4 * addressing review comments, batch #5 * addressing review comments, batch #6
arbeit
added a commit
to arbeit/forseti-security
that referenced
this pull request
Apr 12, 2018
blueandgold
pushed a commit
that referenced
this pull request
Apr 25, 2018
* Added scanner_index_id to violations * Fix failing tests * A first test for the scanner run() function * Second test * minor fix * better test setup * Revert inadvertent change * Add test for init_scanner_index_id() * minor fixes * better name for the new column * added failing test for violation_access.list() * ViolationAccess.list() only returns most recent data The list() method now observes the 'scanner_start_time' column and only returns violations records that were added by the most recent scanner run. * Better tests * Fix pylint warning ************* Module google.cloud.forseti.notifier.notifier R:158, 4: Redefinition of violations type from list to dict (redefined-variable-type) * More complete doc string * Added ScannerIndex class / table * Revert changes to base scanner * Revert changes to scanner builder * Revert changes to notifier * adjust column names/queries * Initialize the 'scanner_index' table before scanner run * fix tests * more test fixes * test & pylint fixes * minor fix * mark scanner_index complete * added test for mark_scanner_index_complete() * better test * review comments, batch #1 * reverse changes to util/db.py * review comments, batch #2 * review comments, batch #3 * review comments, batch #4 * pylint fix * doc string fix * review comments, batch #5 * revert changes to scanners/base_scanner.py * Added test * last_scanner_index() looks at completed db rows by default * better test * added test * reworked violation_access.list() It now distinguishes the following cases: - both the inventory and the scanner index are set - either the inventory or the scanner index is set - neither the inventory nor the scanner index is set * fix tests * Tests for the violation_access.list() method * Handle the case where violation_access.create() is not passed a scanner index. * We want the most recent 'CREATED' scanner index. * Capture result/errors of the scanner run properly * minor fix * review comments, batch #6 * review comments, batch #7 * list() method now supports the inventory index * Tests for ViolationAccess.list() * Proper instantiation of ViolationAccess * Clean up access to 'violation_access' :) * Fix findings test * Fix tests * pylint fixes * fix column name * Flush the sql alchemy session after each scanner * fix column name * call list() with correct param * pylint fixes * morepylint fixes * Expunge violation rows to avoid subsequent db errors * Remove unneeded import * review comments, batch #8 * review comments, batch #9 * fix typo * review comments, batch #10 * pylint fixes * more tests * better error message * Fix the findings result data The scanner_index_id is added but the inventory_index_id needs to stay. * Make code IDE debugger friendly * better query & pylint fixes * better definition of the list() method * better doc string * reviewer changes * changes requested by reviewer * fix tests * add initializer param * convert_to_timestamp() operates on dicts now. Since we do *not* manipulate SQLAlchemy objects any more the need to expunge them from the session is not given any more. * changes requested by reviewer * changes requested by reviewer * changes requested by reviewer * Update notifier.py Change the literal, and updated method doc. * Update notifier.py add blank line in method doc
|
Adding |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This helps to gracefully handle errors when the config files are not created nor have proper values.