-
Notifications
You must be signed in to change notification settings - Fork 123
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
ElasticSearch Update #118
ElasticSearch Update #118
Conversation
Will retry until it can connect to the task db and load storage modules. Fixes mitre#74.
We should use the ElasticSearch recommended doc type of "_doc". From here: https://www.elastic.co/guide/en/elasticsearch/reference/6.x/removal-of-types.html |
Note that "doc" is still used when calling the update API because that's what ES expects: elastic/elasticsearch-py#649
This fixes a bug where API wouldn't work if the one storage module loaded wasn't ES.
# Setup each enabled storage | ||
self.load_modules() | ||
|
||
def load_modules(self, required_module=""): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense for this method to return a module sometimes and a dictionary others?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it only returns a module if required_module
is set.
storage/storage.py
Outdated
|
||
if storage_error: | ||
if required_module: | ||
sys.exit('ERROR: {} module not loaded!'.format(required_module)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe raise our own exception instead of SystemExit
...?
I had a couple of comments about the new method, |
- Use a custom exception instead of sys.exit() - Use values from config file for task database - Ensure required storage module is ENABLED in the config - Make load_modules() always return the same type of object
I like these changes. This looks good to me. |
... since it's used by Elasticsearch but not helpful for the user and is always the same.
Fixes #64, fixes #74, fixes #117, fixes #120.