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

SQLBot draft #1448

Closed
wants to merge 2 commits into from
Closed

SQLBot draft #1448

wants to merge 2 commits into from

Conversation

e3rd
Copy link
Member

@e3rd e3rd commented Sep 6, 2019

OUTDATED: Using SQL connection in a bot can be now more generic. PostgreSQL connection, execution and exception handling code was duplicated. Now, I've pulled it to the PostgreSQLBot that can be inherited instead and that inherits SQLBot.
Like that, we may easily adopt for example SQLite engine which is heavily used in CSIRTMalta workflow.

Could you take look at it please and tell me what design fails you see or that it's alright and I may develop further and do some SQLite bots?

UPDATE: This PR is a draft (just take a look at the code, tests fails due to not be edited yet).

  • I made a class SQLBot in lib/bot.py that handles connection to relational databases – PostgreSQL and SQLite (heavily used in CSIRTMalta workflow). Now, I had two options. Either make a separate expert and output for every relational database (so that user can add "PostgreSQLOutput", "PostgreSQLLookupExpert", "SQLiteOutput", "SQLiteLookupExpert") or make a generic expert and generic output. I've chosen the latter.
  • Any bot using SQLBot will need to have an extra parameter "engine" with the value "postgresql" or "sqlite".
  • Generic DB Lookup is now generic
  • PostgreSQL output is now generic and should be IMHO renamed to SQLOutput

What do you think? That way, we may easily adopt any DB our users need.

Using SQL connection in a bot can be now more generic. PostgreSQL connection, execution and exception handling code was duplicated. Now, I've pulled it to the PostgreSQLBot that can be inherited instead and that inherits SQLBot.
Like that, we may easily adopt for example SQLite engine.
@codecov-io
Copy link

codecov-io commented Sep 9, 2019

Codecov Report

Merging #1448 into develop will decrease coverage by 0.01%.
The diff coverage is 58.33%.

@@             Coverage Diff             @@
##           develop    #1448      +/-   ##
===========================================
- Coverage    75.53%   75.51%   -0.02%     
===========================================
  Files          335      337       +2     
  Lines        16283    16293      +10     
  Branches      2230     2229       -1     
===========================================
+ Hits         12299    12304       +5     
- Misses        3489     3495       +6     
+ Partials       495      494       -1
Impacted Files Coverage Δ
intelmq/lib/sqlite_bot.py 0% <0%> (ø)
intelmq/lib/bot.py 53.05% <61.53%> (-0.41%) ⬇️
intelmq/bots/experts/generic_db_lookup/expert.py 68.42% <75%> (+5.26%) ⬆️
intelmq/bots/outputs/postgresql/output.py 95.23% <83.33%> (+32.19%) ⬆️
intelmq/lib/postgresql_bot.py 83.33% <83.33%> (ø)

this commit should be squashed later, this is just a draft
@ghost
Copy link

ghost commented Sep 10, 2019

What do you think? That way, we may easily adopt any DB our users need.

Cool work, definitely appreciated :)

@ghost ghost added this to the 2.1.0 milestone Sep 10, 2019
@ghost ghost added component: bots component: core feature Indicates new feature requests or new features labels Sep 10, 2019
@e3rd
Copy link
Member Author

e3rd commented Sep 10, 2019

Thanks, I'll do some changes and let you know :)

@e3rd e3rd closed this Sep 10, 2019
@@ -1005,6 +1006,9 @@
"PostgreSQL": {
"description": "PostgreSQL is the bot responsible to send events to a PostgreSQL Database. When activating autocommit, transactions are not used: http://initd.org/psycopg/docs/connection.html#connection.autocommit",
"module": "intelmq.bots.outputs.postgresql.output",
"help": {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we really want to have the help for parameters in that file. (#757 proposes some major steps in how parameters are handled including types and docs/help.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sorry, that's definitely not part of this PR, I've just wondered how to implement help description to the Manager and been designing some ways... Then I discovered #757 and other and saw that nothing is decided yet and gave up :D

@e3rd e3rd changed the title SQLBot SQLBot draft Sep 11, 2019
@e3rd e3rd mentioned this pull request Sep 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: bots component: core feature Indicates new feature requests or new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants