-
Notifications
You must be signed in to change notification settings - Fork 91
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
Improve and extend log_text / log_rules for item logging #605
Conversation
…event issues when parent is not found
…tional option to log_text
…ative items, implement exclude option, minor issue fixing
lib/item/item.py
Outdated
@@ -2018,17 +2018,38 @@ def _log_build_text(self, value, caller, source=None, dest=None): | |||
mlvalue = self._log_mapping.get(lvalue, lvalue) | |||
name = self._name | |||
age = round(self._get_last_change_age(), 2) | |||
pname = self.__parent._name | |||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wozu? _name kann nicht undefiniert sein. _name wird zumindest mit _path initialisiert.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich hatte bei mir mehrfach diesen Error:
2023-11-02 11:41:33 ERROR cherrypy.error.4478625968 [02/Nov/2023:11:41:33] HTTP
Traceback (most recent call last):
File "/Users/ank/Library/Python/3.9/lib/python/site-packages/cherrypy/_cprequest.py", line 638, in respond
self._do_respond(path_info)
File "/Users/ank/Library/Python/3.9/lib/python/site-packages/cherrypy/_cprequest.py", line 697, in _do_respond
response.body = self.handler()
File "/Users/ank/Library/Python/3.9/lib/python/site-packages/cherrypy/lib/encoding.py", line 223, in __call__
self.body = self.oldhandler(*args, **kwargs)
File "/Users/ank/Library/Python/3.9/lib/python/site-packages/cherrypy/_cpdispatch.py", line 54, in __call__
return self.callable(*self.args, **self.kwargs)
File "/Users/ank/Documents/Github/smarthome/modules/admin/itemdata.py", line 346, in item_change_value_html
item(value, caller='admin')
File "/Users/ank/Documents/Github/smarthome/lib/item/item.py", line 1380, in __call__
self.__update(value, caller, source, dest, key, index)
File "/Users/ank/Documents/Github/smarthome/lib/item/item.py", line 2232, in __update
self._set_value(value, caller, source, dest, prev_change=None, last_change=None)
File "/Users/ank/Documents/Github/smarthome/lib/item/item.py", line 2188, in _set_value
self._log_on_change(value, caller, source, dest)
File "/Users/ank/Documents/Github/smarthome/lib/item/item.py", line 2127, in _log_on_change
txt = self._log_build_text(value, caller, source, dest)
File "/Users/ank/Documents/Github/smarthome/lib/item/item.py", line 2021, in _log_build_text
pname = self.__parent._name
AttributeError: 'Items' object has no attribute '_name'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das Problem muss aber wo anders liegen. _name wird in der Init Routine des Items initialisiert (Zeile 184), kann also nicht nicht vorhanden sein.
Der Fehler besagt auch etwas anderes, nämlich dass das das Object Items (nicht das Objekt Item) kein Attribut _name hat. items Ist das Objekt, welches alle Items enthält. Dieses Objekt hat nie ein Atttibut _name.
Hat bei Dir das Item bei dem das Problem auftritt evtl. keinen Parent? Das sollte eigentlich nur für das Wurzel Element des Item-Trees auftreten.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jup ist ein root item.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dann wäre es sinnvoller/verständlicher zu schauen welchen Wert __parent dann hat und das abzufangen, statt auf _name zu prüfen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dann wäre dein Vorschlag mehr in die Richtung..?
if self.__parent = _items_instance:
pname = None
pid = None
else:
pname = self.__parent._name
pid = self.__parent._path
Sonst gerne konkreten Vorschlag posten
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ja
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, ist nun drin
… attributes in log_rules are not set or no log_rules exists
lib/item/item.py
Outdated
|
||
if self._type == 'num': | ||
low_limit = self._log_rules.get('lowlimit', None) | ||
low_limit = self._get_rule('lowlimit') | ||
if low_limit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Zwei Anmerkungen:
- ohne die explizite Abfrage auf
self._type == 'num'
könnte das durchaus auch für strings, bytes o.ä. ohne weitere Änderung gehen. list und dict gehen nicht, und None macht auch Probleme. - Die Abfrage
if low_limit
ist aus meiner Sicht problematisch, weil die if-Bedingung beilow_limit == 0
nicht erfüllt wäre und nicht ausgewertet würde. Besser wäreif low_limit is not None:
. Dito bei high_limit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guter Hinweis, Zweiteres hab ich aktualisiert. Magst du Punkt 1 selbst ändern bzw. klären, warum danach abgefragt wird (so war das ja bereits im Code)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@msinn weißt du, ob es einen bestimmten Grund hat, dass lowlimit und highlimit im Log auf type num beschränkt sind? "Vergleichsfähige" Typen wie str, bytes, bytearray (noch mehr) können ja ohne Codeänderung mit < und > verglichen werden und könnten hier auch verwendet werden...
Ich will nur nicht jetzt Änderungen einbringen, die ggf. aus gutem Grund nicht implementiert wurden ;)
Stimme für "merge", auch in der aktuellen Fassung. Die o.a. Diskussion lässt sich auch gut nach dem Release fortführen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ja, der Grund ist, dass byte und bytearray, etc. keine SmartHomeNG Item-Typen sind.
check if None
These updates should make (together with logging.yaml) datalog, memlog and operationlog obsolete.