[lambda][rule] fixing a bug related to log_pattern enforcement #131
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
ryandeivert
force-pushed
the
ryandeivert-fix-for-log-patterns
branch
from
da68f9b to
cc3a6ce
Compare
jacknagz
reviewed
May 4, 2017
| # configuration settings such as envelope and optional_keys | ||
| converted_data = self._convert_type(data, parser.type(), parser.schema, parser.options) | ||
| converted_data = self._convert_type(data, valid_parse.parser.type(), valid_parse.root_schema, valid_parse.parser.options) |
There was a problem hiding this comment.
rename this to conversion_result
There was a problem hiding this comment.
simplified this - ptal
jacknagz
reviewed
May 4, 2017
| schema_match = False | ||
|
|
||
| for json_record in json_records: | ||
| json_keys = set(json_record.keys()) | ||
| for i in reversed(range(len(json_records))): |
There was a problem hiding this comment.
can we use a different variable name an i
ryandeivert
force-pushed
the
ryandeivert-fix-for-log-patterns
branch
from
42d1d8e to
4a1dcf4
Compare
ghost
suggested changes
May 4, 2017
|
|
||
| from stream_alert.rule_processor.parsers import get_parser | ||
|
|
||
| logging.basicConfig() | ||
| LOGGER = logging.getLogger('StreamAlert') | ||
|
|
||
| # Set the below to True when we want to support multiple matching on multiple schema |
There was a problem hiding this comment.
multiple matching on multiple schema matching
Make this clear
| def _parse(self, payload, data): | ||
| """Parse a record into a declared type. | ||
| def _process_log_schemas(self, payload, data): | ||
| """Get any log schemas that properly classified this log |
There was a problem hiding this comment.
The wording is weird. A schema doesn't classify a log, nor does a schema parse a log (said a bit below). We identify what schemas a log matches to.
There was a problem hiding this comment.
good eye. changed this
ryandeivert
force-pushed
the
ryandeivert-fix-for-log-patterns
branch
4 times, most recently
from
17ebd04 to
a9a1c6b
Compare
* Fixing a bug where typed data was not being returned in records * Recursively nested parsing of csv and json are now supported * Updating tests for multiple schema matching with log_patterns * Updating tests for single schema matching with log_patterns
…keys' and updating docs to reflect the change
ryandeivert
force-pushed
the
ryandeivert-fix-for-log-patterns
branch
from
a9a1c6b to
300a722
Compare
ghost
approved these changes
May 5, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
to: @airbnb/streamalert-maintainers
size: medium
changes
log_patternsto logs that matched only one schema and fixed this so log_patterns will always be applied if they exist for a defined schema.