-
Notifications
You must be signed in to change notification settings - Fork 11
Add new output formats #35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This is required for building with gcc13.
|
||
std::string OITimeToolUserOutputBuilder::dump() const { | ||
std::stringstream ss; | ||
ss << "__RESULT__ " << encodeStatusCode() << " " << userMilliSecondsElapsed_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm so unlike RealTimeOIOutputBuilder, you're doing "__RESULT__ " << encodeStatusCode()
and to be able to do that you're inheriting from OITimeToolOutputBuilder?
Last time we were adding new output builders, we were treating this encoded status code as a legacy thing. It makes the python side of the code more complicated, and IIRC it some error conditions can't be expressed through it.
So unless there's a specific existing tool we want to be a drop-in replacement of (like the original oitimetool binary), we've preferred the oiaug-style format of killReasonName(reason) << " " << exitStatus_
.
Is there an existing parser this needs to be compatible with, or another reason to use encodeStatusCode instead of the oiaug-style format?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason the upstream sioworkers use the default oitt format instead of oiaug, while not utilizing the encoded status code.
I will switch this to oiaug-like format and sometime soon create a PR in sioworkers to use oiaug, as it does indeed allow for simpler code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the upstream sioworkers
uh... um... yeah last time I checked there was quite a bit of differences between upstream sioworkers and prod sioworkers....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally find it funny that there probably does not exist a production deployment with the actual upstream version.
Oiuser is for printing user CPU time, while the second one is human-readable.
oittuser
, as one can guess, is justoitt
but with user CPU time instead of instrucion count.The difference from
oireal
is quite small, but this should allow s2j to be used in place of sioworkers'DetailedUnprotectedExecutor
, allowing a significant security upgrade without having to modify problems' time limits at all.The sys CPU time is also stored, as it might be useful in the future (maybe in sth like a
human-full
output format?).human
provides an oiejq-inspired human-readable output.Resolves #8.
Sample outputs of the latter:
(here the first line is a program's stdout)