-
Notifications
You must be signed in to change notification settings - Fork 372
Allow limiting stdout/err capture #344
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
Comments
While rereading this after receiving #451, I had a realization that did not occur to me at time of posting - namely, are there any downsides to switching to a deque, for the common case of not limiting buffer size? I.e. what do deques sacrifice vs lists? Going by the 2.6 docs, the only downside seems to be that random access in the middle of the structure trends towards O(n), and nearly everything else (appends, pops, etc) is O(1) and/or otherwise an improvement over list performance. So, sounds like the answer is "nope, deques are better in every way unless you're indexing into the middle", which is not necessary in this particular case (and by the time users get their paws on the result, we've concatenated it into a string anyways). But wanted to get this on record. |
Note: this needs care when intersecting with the watchers functionality, since that API is "we give you the current, full copy of the capture buffer on each tick". At the very least we just need a warning block reminding folks that if they configure a limited buffer, they will necessarily only get that buffer in their watchers, and will need to manually reconstruct the entire stream if that's what they really need. (Though this seems...to defeat the point...) |
maybe just a way to indicate where is the new data. in most cases I've seen those care about what is new data, for example echo it into logger, looking for specific lines/strings. |
Description
This is Invoke's version of fabric/fabric#800. tl;dr allow limiting the stdout/err captured, for users who a) experience severe mismatches between the size of their subprocess' output streams and their system's available memory; and b) don't need to examine that output in whole afterwards.
Unlike the notes at the bottom of that ticket, where we're using Fabric 1's
RingBuffer
, here in Invoke land we can just usecollections.deque(xxx, maxlen=yyy)
.Also, because we have a somewhat richer (and publicly documented)
Result
class, we can (and should) add a flag toResult
noting when the ring buffer is being limited, so users have at least a little warning when shooting themselves in the foot ("hey, I turned on this memory saving feature I saw on StackOverflow, but now I only see the last 1024 bytes of stdout!")Specific TODO
deque
objectscapture_buffer_size
(or similarly named) optionmaxlen
for thedeque
The text was updated successfully, but these errors were encountered: