#11710 more disttrial amp errors #11711
Conversation
| errorStreamId = await stream(amp, chunk(error, 2 ** 16 - 1)) | ||
| framesStreamId = await stream(amp, iter(frames)) | ||
| errorStreamId = await stream(amp, chunk(error.encode("utf-8"), 2 ** 16 - 1)) | ||
| framesStreamId = await stream(amp, (frame.encode("utf-8") for frame in frames)) |
There was a problem hiding this comment.
I guess that we assume that we don't have super long lines :)
On a second review: the "frames" which stands for "traceback lines" might not be the best name.
But we can live with it.
There was a problem hiding this comment.
Yea....... The way traceback lines are chunked is not great :/ I was a bit lazy here and didn't try to improve it much. I guess time will tell if it is a safe bet or not.
I agree that frames is not a great name here. Even if we ignore that they're strings and not actual frame objects, they're not even all strings representing frames :/ We are somewhat protected by the fact that they're not even exactly the lines of a traceback either - instead it's "code object names", "filenames", and "line numbers" - and I guess the parent process resolves that info to actual source lines when necessary. These things seem even less likely than source lines to exceed the AMP limit.
Still, someone should probably give this area of the protocol some extra attention. frames is one gross corner but it isn't the only one... :(
| @@ -45,8 +45,8 @@ async def addError( | |||
| :param frames: The lines of the traceback associated with the error. | |||
| """ | |||
|
|
|||
| errorStreamId = await stream(amp, chunk(error, 2 ** 16 - 1)) | |||
| framesStreamId = await stream(amp, iter(frames)) | |||
| errorStreamId = await stream(amp, chunk(error.encode("utf-8"), 2 ** 16 - 1)) | |||
There was a problem hiding this comment.
2 ** 16 - 1 is fine for now.
I guess that in an ideal world, this would be a constant that can be shared between the producer and consumer.
There was a problem hiding this comment.
I'm not sure if I follow. The consumer can always figure this out by looking at the length of the chunks it receives. :)
There was a problem hiding this comment.
True. Thanks for the followup.
I was thinking to have the constant between this code and the AMP protocol.
As this whole chunking work is due to an AMP protocol limitation.
There was a problem hiding this comment.
Ahh yes I see. I think AMP does have a constant for this,
Scope and purpose
Fixes #11710