On Mon, Apr 04, 2016 at 11:13:21AM +0100, 'Viktor Bachraty' via ganeti-devel
wrote:
[snip]
> diff --git a/lib/jqueue/__init__.py b/lib/jqueue/__init__.py
> index 2b90d00..7714d0d 100644
> --- a/lib/jqueue/__init__.py
> +++ b/lib/jqueue/__init__.py
> @@ -621,20 +621,44 @@ class _OpExecCallbacks(mcpu.OpExecCbBase):
> logging.debug("Opcode will be retried. Back to waiting.")
>
> @locking.ssynchronized(_QUEUE, shared=1)
> - def _AppendFeedback(self, timestamp, log_type, log_msg):
> + def _AppendFeedback(self, timestamp, log_type, log_msgs):
> """Internal feedback append function, with locks
>
> - """
> - self._job.log_serial += 1
> - self._op.log.append((self._job.log_serial, timestamp, log_type, log_msg))
> + @type timestamp: tuple (int, int)
> + @param timestamp: timestamp of the log message
Nit: I assume this is seconds from epoch. Also are you sure it's integer, not
float?
> + for msg in log_msgs:
> + self._job.log_serial += 1
> + self._op.log.append((self._job.log_serial, timestamp, log_type, msg))
> self._queue.UpdateJobUnlocked(self._job, replicate=False)
Nit: Do we really need to increment the serial number per message, or could
we just do it once overall? I know what you've got preserves the current
behaviour, but from luxid's point of view, isn't it only going to care that
the new serial > old serial?
[snip]
> + self._feedback_fn = self.Feedback
> self.bad = False
>
> + # TODO: Cleanup calling conventions, make them explicit
> + def Feedback(self, *args):
> +
> + # TODO: Remove this once calling conventions are explicit.
> + # Feedback can be called with anything, we interpret ELogMessageList as
> + # messages that have to be individually added to the log list, but pushed
> + # in a single update. Other types are only transparently passed forward.
> + if len(args) == 1:
> + log_type = constants.ELOG_MESSAGE
> + log_msg = args[0]
> + else:
> + log_type, log_msg = args
> +
> + if log_type != constants.ELOG_MESSAGE_LIST:
> + log_msg = [log_msg]
Any reason not to make this if log_type == constants.ELOG_MESSAGE: ?
> +
> + self.msglist.extend(log_msg)
[snip]
> + # Feedback can be called with anything, we interpret ELogMessageList as
> + # messages that have to be individually added to the log list, but pushed
> + # in a single update. Other types are only transparently passed forward.
> + if log_type == constants.ELOG_MESSAGE_LIST:
> + log_msg_list = [(constants.ELOG_MESSAGE, msg) for msg in log_msg]
> + else:
> + log_msg_list = [(log_type, log_msg)]
As above.
