Skip to content

Conversation

@victorusu
Copy link
Contributor

This PR adds an additional field to the LoggerAdapter class as suggested by @vkarak. This new field holds the original UNIX time and it is used to format the check_job_completion_time field.

Fixes #1261
Fixes #1265

@victorusu victorusu requested review from ekouts, teojgo and vkarak April 23, 2020 09:44
@victorusu victorusu changed the title Fix string conversion in the RFC3339Formatter [bug] Fix string conversion in the RFC3339Formatter Apr 23, 2020
@vkarak vkarak changed the title [bug] Fix string conversion in the RFC3339Formatter [bugfix] Fix string conversion in the RFC3339Formatter Apr 23, 2020
@vkarak vkarak added this to the ReFrame sprint 20.06 milestone Apr 23, 2020
Copy link
Contributor

@ekouts ekouts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix is working however there are two things missing or need more thinking:

  1. We will need a unit test to reproduce the bug.
  2. The check_job_completion_time is not sent over to Graylog. Only the raw time is sent. I don't know if we can do something for that, because what is sent to Graylog are the extras and not the formatted text. I don't know if formatting the message and sending it is the correct way to deal with that or formatting at the Graylog site is the best solution, but I kind of think the second one is at least more correct.

Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add an entry in the documentation explaining the new field?

@vkarak vkarak changed the title [bugfix] Fix string conversion in the RFC3339Formatter [bugfix] Fix logging crash when increasing verbosity level Apr 28, 2020
@vkarak
Copy link
Contributor

vkarak commented Apr 28, 2020

@victorusu I've updated the documentation.

@vkarak vkarak merged commit 9ef74ac into reframe-hpc:master Apr 28, 2020
@victorusu victorusu deleted the bug/RFC3339Formatter branch October 6, 2020 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ReFrame crashes if verbosity level is increased logging inside regression test

3 participants