I thought we'd discussed this already, but I think having the same index for all three is a bad API design. I think the use cases are generally separate and conflating them effectively reduces the size of the arrays. If I could, I'd change the TXN and SSN args to use separate indices and would be happy to make a PR that does that. I suspect there is not even one plugin that depends on that behavior.
On Tue, Nov 14, 2017 at 1:18 AM, Leif Hedstrom <zw...@apache.org> wrote: > > > > On Nov 8, 2017, at 11:08 PM, Alan M. Carroll < > a...@network-geographics.com> wrote: > > > > This came up with issues #2380 and #2388 and PR #2783. I had been > waiting for some internal feedback on my proposal but since this is now > active I am sending in my API proposal for attaching plugin data to > NetVConnections (TSVConn). > > > > https://solidwallofcode.github.io/api/TSVConnArgs.en.html#tsvconnargs > > > > Some background on this proposal > > > > https://solidwallofcode.github.io/vconn-args.en.html > > > > > I redact my +1 :-). > > It seems we used one “index” lookup / storage for TXN and SSNs. Are we > sure we want a separate lookup function and table for the TSVConn? That > seems inconsistent. I think if we’re going to do this, we should break > compatibility on the old SSN, and break that out of all of this. I.e. make > > TSHttpSsnArgIndexReserve > > and > > TSHttpTxnArgIndexReserve > > > etc. Otherwise, the proposal here seems very inconsistent with the > existing APIs, to the point of being confusing as hell. We should either > change the new proposal to reuse the same index slots as previous (they > really are per Plugins anyways), or we should fix the old APIs IMO. > > — Leif > >
