Code review comment for lp:~mgorven/ibid/oui

Revision history for this message
marcog (marco-gallotta) wrote :

Query: reload sysadmin
DEBUG:core.reloader:Loading Processor: sysadmin.AptFile
ERROR:core.reloader:Couldn't instantiate AptFile processor of sysadmin plugin
Traceback (most recent call last):
  File "./ibid/core.py", line 235, in load_processor
    ibid.processors.append(klass(name))
  File "./ibid/plugins/__init__.py", line 80, in __init__
    self.setup()
  File "./ibid/plugins/sysadmin.py", line 118, in setup
    raise Exception("Cannot locate apt-file executable")
Exception: Cannot locate apt-file executable
Response: Couldn't reload sysadmin

Not sure if we usually implement workarounds for this, but this is failing because another feature in this plugin has a dependency that isn't installed.

+ @match(r'^(?:(?:mac|oui|ether|ether(?:net)?(?:\s*code)?)\s+)?((?:(?:[0-9a-f]{2}[:-]?){3}){1,2})$')

You can remove the "ether|" as it's caught by the one that follows.

Do you want to allow a trailing [:-]? (e.g. 11:22:33: matches)

Query: 112233
Response: I don't know who that belongs to

That is very unexpected if the user was not trying to lookup a mac address. I would require the 11:22:33 format (even disallowing 11-22-33 because that looks like an expression) if the leading word (mac, ether, etc.) is left out.

review: Needs Fixing

« Back to merge proposal