Logged notebook type #5425
Logged notebook type #5425
Conversation
There was a problem hiding this comment.
Great idea @pinarkavak - thank you. I just had some minor comments.
| @@ -177,6 +177,7 @@ async def start_kernel(self, kernel_id=None, path=None, **kwargs): | |||
| self._kernel_connections[kernel_id] = 0 | |||
| self.start_watching_activity(kernel_id) | |||
| self.log.info("Kernel started: %s" % kernel_id) | |||
| self.log.info("Kernel type: %s" % self._kernels[kernel_id].kernel_name) | |||
There was a problem hiding this comment.
Rather than introduce another INFO entry that might result in ambiguities when starting kernels simultaneously, would you be okay with extending Kernel started: <id> with a comma-separated "type" clause: Kernel started: <id>, type: <type>?
There was a problem hiding this comment.
Hmm - wondering if name: might be more indicative and closer to what we use in the code?
There was a problem hiding this comment.
Thanks Kevin, great points, my first intention was not to change any existing log, but updated the code to address your comments.
| @@ -176,7 +176,7 @@ async def start_kernel(self, kernel_id=None, path=None, **kwargs): | |||
| kernel_id = await maybe_future(self.pinned_superclass.start_kernel(self, **kwargs)) | |||
| self._kernel_connections[kernel_id] = 0 | |||
| self.start_watching_activity(kernel_id) | |||
| self.log.info("Kernel started: %s" % kernel_id) | |||
| self.log.info("Kernel started %s, name: %s" % (kernel_id, self._kernels[kernel_id].kernel_name)) | |||
There was a problem hiding this comment.
Thank you for the updates Pinar. I hate to nit, but I think the : following started should probably be retained. The only reason is in case there are "log scrappers", they'll probably be looking for Kernel started: - which is along the lines of your thoughts about preserving log entries. I suspect any applications that perform log analysis and need the kernel start time would be targeting "Kernel started:" as their token and not so much the content to EOL.
There was a problem hiding this comment.
Thanks Kevin! I totally agree, I somehow removed it without noticing while updating the log, added it back.
pinarkavak
force-pushed
the
log-notebook-type
branch
from
9b93626
to
8c063c2
Compare
No description provided.