apache / kibble-1

Apache Kibble - a tool to collect, aggregate and visualize data about any software project
https://kibble.apache.org/
Apache License 2.0
58 stars 27 forks source link

Refactor KibbleDatabase #84

Open turbaszek opened 3 years ago

turbaszek commented 3 years ago

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: #85
turbaszek commented 3 years ago

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 commented 3 years ago

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

turbaszek commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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