Skip to content

Conversation

@SolidWallOfCode
Copy link
Member

This is not intended to be merge immediately because it describes API that has not yet been written. Comments should be about the API itself.

The new functions are
TSIOBufferReaderRead
TSIOBUfferReaderIterate
TSHttpTxnErrorBodyBufferSet
TSHttpTxnApplyLogFormat

The other documentation is of API that is already present but undocumented. Feel free to comment in that regard.

@SolidWallOfCode
Copy link
Member Author

I added the new function TSIOBufferReaderIsAvailAtLeast because this has proved handy in the core and (since it's an existing method) this will be easy to implement. I also cleared up how blocks are consumed by TSIOBufferReaderIterate.

@jpeach
Copy link
Contributor

jpeach commented Oct 14, 2015

TSHttpTxnApplyLogFormat

  • I think the name could be clearer, maybe TSHttpTxnFormatLogTags() or TSHttpFormatTags().
  • It seems reasonable to format to an IO buffer, but if you think
    you would like to also be able to format to a pre-allocated buffer
    (pointer + length), we should choose the right name now so that
    later they will name sense.

TSHttpTxnErrorBodyBufferSet

  • The description text doesn't seem to match.
  • Since this is clearly intended to be paired with the log formatter,
    it would be helpful to explicitly join the dots in the description.
  • I don't think you need the MIME type argument since it is easy
    to set the Content-Type header using existing API. If you think
    the existing API is too troublesome, maybe a secondary helper API
    that can set a named header?

You should go ahead and commit the TSIOBufferReaderAlloc man page,
but call it TSIOBufferReader since it is documenting the whole API
family (which makes sense).

@SolidWallOfCode
Copy link
Member Author

@bryancall
Copy link
Contributor

@SolidWallOfCode said he would work on it this quarter.

@zwoop
Copy link
Contributor

zwoop commented Mar 22, 2016

@SolidWallOfCode You going to land this in this incarnation? Or another PR?

SolidWallOfCode pushed a commit to SolidWallOfCode/trafficserver that referenced this pull request Feb 1, 2017
YTSATS-927: Have requests over the server_max_connections limit fail …
SolidWallOfCode pushed a commit to SolidWallOfCode/trafficserver that referenced this pull request May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants