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

Sort, and HTML digest. #3

Merged
merged 7 commits into from Mar 16, 2019

Conversation

Projects
None yet
2 participants
@ethauvin
Copy link
Contributor

ethauvin commented Mar 11, 2019

Quite a few changes.

First, fixed a bug where the db_creation_date_key was never found in the DB keys. It was not UTF-8 encoded.

Second, fixed the sorting. While there was technically nothing wrong with sorting by the number of occurrences, it created some weird behavior in most cases, for example:

  1 event(s) for IP 45.55.160.243                             : 2019-03-04 07:24:50
  1 event(s) for IP 159.89.116.97                             : 2019-03-04 07:06:08
  1 event(s) for IP 37.97.129.58                              : 2019-03-04 07:25:59
  1 event(s) for IP 68.183.76.110                             : 2019-03-04 06:56:15
  1 event(s) for IP 188.166.10.79                             : 2019-03-04 07:09:32
  1 event(s) for IP 66.70.188.25                              : 2019-03-04 06:38:39
  1 event(s) for IP 139.59.82.59                              : 2019-03-04 06:09:39
  1 event(s) for IP 178.62.8.222                              : 2019-03-04 07:38:59
  1 event(s) for IP 45.55.41.232                              : 2019-03-04 07:17:34
  1 event(s) for IP 104.131.93.33                             : 2019-03-04 07:20:19
  1 event(s) for IP 178.128.79.169                            : 2019-03-04 06:02:10
  1 event(s) for IP 138.68.146.186                            : 2019-03-04 07:22:52
  1 event(s) for IP 211.250.189.64                            : 2019-03-04 06:08:28
  1 event(s) for IP 14.232.202.166                            : 2019-03-04 06:03:38

Since all the events are 1, it basically creates a radom sort. I fixed the problem by first sorting by the event date then by occurrences. I've also added a --sort argument to turn sorting by occurences on or off.

Last, I've added an HTML template for the digest, which can be turned on using the --html argument. It is off by default. I just find the plain text template is hard to read in Gmail, etc.

Screenshot_20190310-172601

I think it could be turned on by default since both the plain text and HTML template are used in the email digest, but I did not want to change the current behavior.

@enricotagliavini Let me know if you have any questions/comments/suggestions.

@@ -110,7 +110,7 @@ def db_busy_open(filename, flags, timeout):

def add(db, ip):
db = db_busy_open(db_location + '/' + db + '.dbm', 'c', 30)
if db_creation_date_key not in db.keys():
if db_creation_date_key.encode('UTF-8') not in db.keys():

This comment has been minimized.

@enricotagliavini

enricotagliavini Mar 12, 2019

Owner

What about adding "u" in front of the string at line # 37 instead?

db_creation_date_key = u'db_creation_date'

Should have the same effect should it not?

This comment has been minimized.

@ethauvin

ethauvin Mar 12, 2019

Author Contributor

It should. I just made the change on my system and will let it run for a bit to double check. I'll update the PR if it works as it should.

@enricotagliavini

This comment has been minimized.

Copy link
Owner

enricotagliavini commented Mar 12, 2019

Hi Erik,

sorry I fat fingered the keyboard and closed the pull request by accident, I'm reopening it now.

All good suggestions. Thank you for not changing the default behavior, I appreciate that, but I understand different sorting and HTML alternative is desirable. I just tried in gmail and it indeed looks not aligned correctly as they don't use a fixed width fonts.

I'll try it on my system this evening and it's probably good to be merged. I also commented on one line in one of the commits, let me know what you think about it.

Thank you very much for the contribution.
Kind regards.

@enricotagliavini enricotagliavini merged commit e774bec into enricotagliavini:master Mar 16, 2019

@enricotagliavini

This comment has been minimized.

Copy link
Owner

enricotagliavini commented Mar 16, 2019

Merged, thank you very much for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.