Arch Linux

archforum-asdffdsa

Member

Registered: 2013-06-18

Posts: 3

journalMessenger

I've made a small python program, which alters the input it gets from systemd's journal or from stdin according to certain rules and outputs it to libnotify or stdout
Further explaination and the code can be found on github
Since this is my first real program, I would really appreciate feedback, critics as well as improvements

Stebalien

Member

Registered: 2010-04-27

Posts: 1,239

Website

Re: journalMessenger

A couple of starter points:
* Try using argparse instead of getopt.
* The convention in python is to use 4 spaces instead of tab characters.
* Writing your own rule file config syntax is overkill, error-prone, and likely to confuse users. For your program, I would recommand yaml or JSON.
* Comment.
* The journal module allows you to add custom filters. If you could use those instead of manually filtering every line, your script would be more efficient.

---

Steven [ web : git ]
GPG:  327B 20CE 21EA 68CF A7748675 7C92 3221 5899 410C

archforum-asdffdsa

Member

Registered: 2013-06-18

Posts: 3

Re: journalMessenger

So I've worked on it a bit

* Try using argparse instead of getopt.

Thanks, I didn't know that! It's easier to use, thanks. I use argparse now

* The convention in python is to use 4 spaces instead of tab characters.

...So isn't tab easier to use? (1 character vs. 4). Anyhow, I'm probably gonna change that in near future

* Writing your own rule file config syntax is overkill, error-prone, and likely to confuse users. For your program, I would recommand yaml or JSON.

Again: Thanks! In retrospective, that seemed completely obvious! Anyhow, now i use json. For the profiles, I still use my own syntax, since json in inappropiate and the syntax is realy easy (to implement as well as to use)

* Comment.

Better so?

* The journal module allows you to add custom filters. If you could use those instead of manually filtering every line, your script would be more efficient.

But it would take away the flexibility it has now. As it is now, the rules can be shared, and writing your own profile is extremely easy!

I hope this can be usefull, and i'm probably going to add a tool to make rule-creating easier.

Last edited by archforum-asdffdsa (2013-06-23 20:18:40)

Stebalien

Member

Registered: 2010-04-27

Posts: 1,239

Website

Re: journalMessenger

Really Nice.

About the tabs etc, styling conventions are there to make it (a) easier to understand someone else's code and (b) easier to share code.  While tabs are great as they theoretically let everyone set their own indentation level width, there are cases where one needs to indent a set number of characters (using spaces). This mixing of tabs and spaces can get out of hand really quickly. Furthermore, if you are using a more reasonable tab width (the standard, 8, is too wide IMHO), the mixture of tabs and spaces leads to really ugly indentation.

Here are the python style convention pages:
  Style: http://www.python.org/dev/peps/pep-0008/
  Documentation: http://www.python.org/dev/peps/pep-0257/

More code notes:

I would use str.split() in rules.applyRule instead of manually finding the space.

for command in self.values[valueName]:
    command_pieces  = command.split(' ', 1)
    command_keyword = command[0]
    if command_keyword == 'read':
        ...
    elif command_keyword == 'regex':
    ...

Also, if you want to be fancy, you can write separate handlers for each command (overkill but more extensible):

# All handlers named handle_<cmd>
def handle_read(self, outputDict, value, argument):
    ...
for command in self.values[valueName]:
    command_pieces  = command.split(' ', 1)
    try:
        command_fn = getattr(self, "handle_"+command_pieces[0])
    except AttributeError:
        sys.stderr.write('Error: rule "' + self.name + '": command "' + command + '" does not exist.\n')
        valueStrings[valueName] = ''
    else:
        # If an exception is not thrown
        valueStrings[valueName] = command_fn(outputDict, valueStrings[valueName], *command_fn[1:]) # *command_fn passes the second part of `command_pieces` if it exists

Last edited by Stebalien (2013-06-25 13:31:16)

---

Steven [ web : git ]
GPG:  327B 20CE 21EA 68CF A7748675 7C92 3221 5899 410C