[devel] feat: implement rolling pilot log output

[marianne013]

@fstagni @martynia
Simon had a go at trying to make the pilot write its log file on a rolling basis. I tested it and as far as I can tell it doesn't break anything.
This might be a good stepping stone towards handling the output in the pilot logger.

[marianne013]

@sfayer Forgot to tag you.

[martynia]

I'm getting the full command buffer from the command execution at the moment, but it could be modified, I think. I can flush my buffer on demand as well, not after certain number of lines.

[marianne013]

@sfayer @fstagni Simon and me were debugging some LHCb pilots (as it happens ;-) using 0 CPU yesterday and we could have really done with a rolling log output. Would you consider approving this pull request as an interim measure until the pilot logger is finished ?

[fstagni]

I can, as anyway this is targeting devel branch and so it will go through a hackathon. It's still a draft though.

[marianne013]

I'll deal with it after my next meeting, but before the end of today.

[sfayer]

@marianne013: from testing, we think this patch is also needed:

             sys.stdout.write("\n")                                                                                                                                                                                                                                         
             sys.stdout.flush()                                                                                                                                                                                                                                             
+            sys.stderr.write("\n")                                                                                                                                                                                                                                         
+            sys.stderr.flush()

Regards,
Simon

[marianne013]

I was kind of hoping it would let me click the "implement suggestion" button, but alas, no

[martynia]

The code works in"real life" in our test v7r3 installation, however I'm not sure if python 2 is not a limitation in this scenario.

[martynia]

Since outData and all the chunks are strings, outData.decode will not work in Python 3. I added a little unit test in #158, which passes when run in Python2 only.

[marianne013]

Can you be slightly more precise as in which test is it that fails and what the error is ?

[martynia]

You would need to check #158 out, and then:

python2 -m pytest tests/CI/Test_simplePilotLogger.py

this would pass. If you run the same test with Python 3 it will fail. This is because outData is a string and doesn't have a decode() method in Python 3. You cannot change everything to bytes to satisfy line #428, because writing to stdout/stderr requires strings (add appending chunks to outData requires strings), so outData must be a string.

You can simply run in python 2 and 3 and see what happens:

text="Hello World"
text.decode('acsii','replace')

[martynia]

I extended my test to run it for a bunch of strings with different lengths (not pushed yet). It seemed to run fine, but after I repeated the test for a few times (from a command line, as shown above), I got a failure. It was somewhat difficult to see where the strings were different (at 1025 length...). A closer look (text is a random ASCII letter sequence + \n ) \

• the original string starts with a newline character, the string processed by the program doesn't (it is now 1024 characters long)

So it is missing a leading newline character. Not the end of the world, we can live with it, I can exclude these cases from a test.

[sfayer]

Where are we with this? We're really keen to get this rolling output in as it's hampering our attempts to debug other problems as we have to wait for the JobAgent to finish to get its log output...

Regards,
Simon

[martynia]

Can you live with the fact, that the code does not work with Python 3 ? See my comments above to Daniela's post.

[fstagni]

Can you live with the fact, that the code does not work with Python 3 ? See my comments above to Daniela's post.

no :)

[martynia]

              outChunk = stream.read(1024).decode("ascii", "replace")

E AttributeError: 'str' object has no attribute 'decode'

stream.read() returns an str apparently, so the problem persists in Python 3, doesn't it ?

[sfayer]

              outChunk = stream.read(1024).decode("ascii", "replace")

E AttributeError: 'str' object has no attribute 'decode'

stream.read() returns an str apparently, so the problem persists in Python 3, doesn't it ?

Nah, stream.read() is bytes on py3, str on py2 (which is how the old code works with both)... We're still running into a few minor unicode problems, but we think that should generally fix it.

[sfayer]

We've re-tested this latest version with both py3 & py2 and it works for both.... Any other comments?

Regards,
Simon

[martynia]

Tested with Python 2 and 3. OK.

[sfayer]

Hi @fstagni,

I think this PR is all ready to go now, do you need anything further from us to get it merged?

Regards,
Simon

[martynia]

I think we could go ahead. I’ll deal with this change in my remote logger, once it is in. Cheers, JM

…

On 21 Jun 2022, at 15:57, fstagni ***@***.***> wrote: I can, as anyway this is targeting devel branch and so it will go through a hackathon. It's still a draft though. — Reply to this email directly, view it on GitHub <#155 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB2KE7K2WTPHQDSXL46EDKDVQHC3RANCNFSM5XD6R3QQ>. You are receiving this because you were mentioned.