Martin Peřina has posted comments on this change. Change subject: core: Introduce standalone fence_kdump listener ......................................................................
Patch Set 3: (7 comments) http://gerrit.ovirt.org/#/c/27201/3/packaging/services/ovirt-fence-kdump-listener/db.py File packaging/services/ovirt-fence-kdump-listener/db.py: Line 2: Line 3: import logbase Line 4: Line 5: Line 6: @logbase.Logable() > please use the base.Base as base for all objects I can't inherit from base.Base: 1) It causes problems with constructor calling in classes that inherits from threading.Thread or SocketServer.DatagramRequestHandler 2) Logger defined in base.Base is configured to log to syslog, but I'd like listener logic to be logged in its own log file under /var/log/ovirt-engine Line 7: class Manager(object): Line 8: def __init__( Line 9: self, Line 10: host, Line 15: secured, Line 16: secure_validation, Line 17: ): Line 18: self._db_dsn = ( Line 19: "host=%s port=%s dbname=%s user=%s password=%s sslmode=%s" % ( > please see[1], and Statement as a while of how to create and use connection There's no way how to just import database.py into service, right? I will have to copy Statement class here, right? Line 20: host, Line 21: port, Line 22: database, Line 23: username, Line 26: ) Line 27: ) Line 28: self._config = {} Line 29: Line 30: @staticmethod > you do not really need staticmethod for private functions unless there is a Well, I don't use anything from instance in method, so Idea suggested me to make this method static ... Line 31: def _get_ssl_mode(secured, secure_validation): Line 32: sslmode = "disable" Line 33: if secured == "True": Line 34: if secure_validation == "True": http://gerrit.ovirt.org/#/c/27201/3/packaging/services/ovirt-fence-kdump-listener/flowterminator.py File packaging/services/ovirt-fence-kdump-listener/flowterminator.py: Line 8: import logbase Line 9: Line 10: Line 11: @logbase.Logable() Line 12: class KdumpFlowTerminator(threading.Thread): > there is no need to use thread, please use async io. I searched for asynio module and it exists only for Python 3.4. Or did you mean something else? Line 13: """Creates record with status "finished" for finished kdump flows""" Line 14: Line 15: def __init__(self, queue, db_manager): Line 16: super(KdumpFlowTerminator, self).__init__() Line 53: while True: Line 54: try: Line 55: conn = self._db_manager.open_connection() Line 56: Line 57: finished = datetime.datetime.now(dateutil.tz.tzlocal()) > always use utc, never localtime, unless you present something to user. Well, all timestamp columns in engine db are defined as 'TIMESTAMP WITH TIMEZONE', so I created fence_kdump_messages similarly. So I need to use timezone aware datetime also here in order the db value to be correct. Am I right? Line 58: records = KdumpFlowTerminator._get_unfinished_flows(conn) Line 59: for record in records: Line 60: if ( Line 61: record["received"] + self.kdump_finished_timeout Line 80: finally: Line 81: self._db_manager.close_connection(conn) Line 82: conn = None Line 83: Line 84: time.sleep(30) > always have parameters for these, even invent vdc_option and set 30 as defa Every 30 sec the thread looks for unfinished kdump flows and if last update is older than 30 seconds (KdumpFinishedTimeout), it makes the flow finished. More info at [1], but I can add this to vdc_options. [1] http://www.ovirt.org/Fence_kdump#New_standalone_fence_kdump_listener http://gerrit.ovirt.org/#/c/27201/3/packaging/services/ovirt-fence-kdump-listener/heartbeat.py File packaging/services/ovirt-fence-kdump-listener/heartbeat.py: Line 32: def run(self): Line 33: conn = None Line 34: while True: Line 35: try: Line 36: conn = self._db_manager.open_connection() > please use single connection for the entire application. AFAIK there's no transaction manager like in J2EE, so if the single connection would be used for all threads and error will appear only in 1 thread and I would execute rollback on connection, wouldn't it affect all other threads using this connection? Line 37: Line 38: Heartbeat._update_heartbeat( Line 39: conn, Line 40: datetime.datetime.now(dateutil.tz.tzlocal()), -- To view, visit http://gerrit.ovirt.org/27201 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieec3bad47bbba860a52a9ff4e2eb7f61277f4e36 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
