Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
From Linux bash script to Python control flow #386
From Linux bash script to Python control flow #386
Changes from 7 commits
e417791
53c00a0
31339ba
a052c71
850d109
dd4fc69
fefe128
68d8f89
1d93336
df69dda
2edb336
762dc80
c956f11
ad871f8
91c3e90
faf6a76
27d97da
e31c24d
b12a560
cc913ee
833a069
9e35d65
4c0af9f
44e3198
5842a75
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Could we somehow use
\0
to mark the end of a message? To be fair, I don't know how bash handles this.At any rate, do we need to generate a UUID each time? I'd think we could just take a fixed string marker?
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.
There's nothing stopping a child process from outputting
\0
and causing havoc.Normally, when doing this kind of protocol, there'd be a header that specifies the length of the following message -that makes reads really easy. But I wanted to avoid buffering the output on the container side, for memory reasons but mostly so that output still streamed back to the console - during a long build this is very reassuring, or helpful if a command stalls. This solution was inspired by SMTP multipart boundaries - basically a random string long enough is unique enough to delimit between parts of data. UUID is just an easy way to make a unique string.
I could add some of this context as a comment I guess?
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.
Alright, yes. Good point! I hadn't considered that, yet, but this is an important feature, then! :-)
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.
We could still just define our own constant magic end-of-message UUID, no? Is there a reason to introduce randomness and regenerate a new one every time?
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 think there's something comforting about using a new UUID each time - it's a string that's never existed in the world before. but practically, err, the most concrete I can get with it is that if it was static and written in code, and then somehow this code file was sent down the pipe, then the terminator would appear before the end of the message. Far-fetched, but that's the kind of thing I'm thinking about. But a fresh UUID has never existed before, so that kind of thing couldn't happen.
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.
That's a bit far-fetched, but it had completely not crossed my mind! Better safe than sorry, then :-)