Alon Bar-Lev has posted comments on this change. Change subject: core: Introduce standalone fence_kdump listener ......................................................................
Patch Set 3: (6 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() > I can't inherit from base.Base: 1. you should not use threading. 2. no... services should go to syslog. 3. you can add your own appender in addition if it is that important... but never override our setting, these are there for a reason. 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" % ( > There's no way how to just import database.py into service, right? I will h 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 > Well, I don't use anything from instance in method, so Idea suggested me to for private functions there should be no difference, just more noise on source. 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): > I searched for asynio module and it exists only for Python 3.4. Or did you use plain poll. see example[1], and you have datagram which is much easier to process. [1] http://gerrit.ovirt.org/gitweb?p=otopi.git;a=blob;f=src/otopi/plugin.py;hb=HEAD#l337 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()) > Well, all timestamp columns in engine db are defined as 'TIMESTAMP WITH TIM database value should be utc, there is no sense of keeping timezone within database, unless you are using calendar for people. 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 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() > AFAIK there's no transaction manager like in J2EE, so if the single connect if you do not use threads, then you can have your transaction at each operation without any complexity. 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 <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
