Code review comment for lp:~jk0/nova/xs-rescue-periodic-tasks

Revision history for this message
Sandy Walsh (sandy-walsh) wrote :

9 +def is_then_greater(then, seconds):
40 + if utcnow() - then > datetime.timedelta(seconds=seconds):
41 + return True
42 + else:
43 + return False

Beyond the naming as matt pointed out, how about

return utcnow() - then > datetime.timedelta(seconds=seconds)

98 + continue
log message?

105 + if _vbd_ref["userdevice"] == "1":

Is there a risk of userdevice not being defined?
Perhaps, if _vbd_ref.get("userdevice", None) == "1":

173 + if last_ran:
174 + if not utils.is_then_greater(last_ran, timeout * 60 * 60):
175 + # Do not run. Let's bail.
176 + return
177 + else:
178 + # Update the time tracker and proceed.
179 + self.poll_rescue_last_ran = utils.utcnow()
180 + else:
181 + # We need a base time to start tracking.
182 + self.poll_rescue_last_ran = utils.utcnow()
183 + return

I try to keep my returns together to keep the if blocks smaller

173 + if not last_ran:
181 + # We need a base time to start tracking.
182 + self.poll_rescue_last_ran = utils.utcnow()
183 + return

174 + if not utils.is_then_greater(last_ran, timeout * 60 * 60):
175 + # Do not run. Let's bail.
176 + return

177 + # Update the time tracker and proceed.
179 + self.poll_rescue_last_ran = utils.utcnow()

No tests?

review: Needs Fixing

« Back to merge proposal