Skip to content
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

Refactor KibbleDatabase #84

Open
turbaszek opened this issue Oct 27, 2020 · 5 comments
Open

Refactor KibbleDatabase #84

turbaszek opened this issue Oct 27, 2020 · 5 comments
Assignees

Comments

@turbaszek
Copy link
Member

turbaszek commented Oct 27, 2020

Currently we implement two times the same class: KibbleDatabase:

https://github.com/apache/kibble/blob/2abfcc871dd35ddc727317267a4595f8230b53eb/kibble/setup/makeaccount.py#L27

https://github.com/apache/kibble/blob/2abfcc871dd35ddc727317267a4595f8230b53eb/kibble/api/plugins/database.py#L121

What should be done:

  1. We should consolidate the whole logic into single class and create kibble/database.py that will keep definition of this object.
  2. Refactor this class to use values from KibbleConfigParser from kibble/configuration.py
  3. Drop support for es < 7 as per: Support only Elasticsearch 7+ #85
@turbaszek
Copy link
Member Author

Additionally when working on #94 I got the following warning. It would be good to address it:

/usr/local/lib/python3.8/site-packages/elasticsearch/connection/base.py:190: ElasticsearchDeprecationWarning: [types removal] Specifying types in document index requests is deprecated, use the typeless endpoints instead (/{index}/_doc/{id}, /{index}/_doc, or /{index}/_create/{id}).
  warnings.warn(message, category=ElasticsearchDeprecationWarning)

@skekre98
Copy link
Contributor

Hi @turbaszek, I can take a look into this if it is still available?

@turbaszek
Copy link
Member Author

@skekre98 sure! I did some changes in #94 around this. However what we need is to:

  1. Figure out where we use this class (= where we access the database)
  2. Decided what is the best approach to refactor it

@turbaszek
Copy link
Member Author

Doing #114 I saw that we have also similar logic of creating es connection in:
https://github.com/apache/kibble/blob/5bf37a8c0db83c918fa476a3a1b653390026e169/kibble/scanners/brokers/kibbleES.py#L286-L298

@kaxil
Copy link
Member

kaxil commented Dec 13, 2020

Yeah there is lot of duplication which cna be simplified.

Same with yaml file used by scanners to, I think we can unify those configs in the top level .ini file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants