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

Sybenzvi/csv to sql #106

Merged
merged 15 commits into from
Aug 14, 2024
Merged

Sybenzvi/csv to sql #106

merged 15 commits into from
Aug 14, 2024

Conversation

sybenzvi
Copy link
Contributor

This PR addresses issue #98 by doing the following:

  • Removes the 24-hr and 7-day CSV alert caches and logs.
  • Implements connection to an SQLite disk file using sqlalchemy
  • Automatically removes alerts older than the config/environment setting HB_DELETE_AFTER

Note that the use of sqlalchemy allows us to switch to a different DB in the future (e.g., Postgres) if we decide to do that, with only a trivial change to the sqlalchemy engine object.

@sybenzvi sybenzvi added the enhancement New feature or request label Aug 13, 2024
@sybenzvi sybenzvi requested a review from KaraMelih August 13, 2024 21:03
@sybenzvi
Copy link
Contributor Author

It looks like we need a unit test for this.

Copy link
Collaborator

@KaraMelih KaraMelih left a comment

Choose a reason for hiding this comment

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

I think, I am happy with this version. Database works as expected, I added a temporary fix for the factorial problem mentioned here #100

I updated the poetry dependencies to include sqlalchemy and tabulate

tabulate is used when someone requests a

snews_pt write-hb-logs

then it dumps the table on the console (on the server)

@KaraMelih
Copy link
Collaborator

One thing that might not be desirable is the precision of the Latencies.
Here;

msg["Latency"] = msg["Latency"].astype('timedelta64[s]').astype(int)

I converted them to integer seconds because sqlalchemy did not like the timedelta objects. However, we are loosing some precision. We should decide if want to improve that

@sybenzvi sybenzvi merged commit e29b8cc into main Aug 14, 2024
@sybenzvi sybenzvi deleted the sybenzvi/csv-to-sql branch August 14, 2024 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants