Introduce simplified parsers #2972
Conversation
|
This has 100% test coverage where 90% of those tests were copied from the previous package. |
|
@iblancasa can do :D I want to land this, then do exporters, then do processors and then migrate over to use this. You can see an example of the usage here |
| if r.MatchString(endpoint) { | ||
| portStr := r.FindString(endpoint) | ||
| cleanedPortStr := strings.Replace(portStr, ":", "", -1) | ||
| port, err = strconv.ParseInt(cleanedPortStr, 10, 32) |
There was a problem hiding this comment.
Port numbers are 16-bit integers, so this should really be 16 instead of 32. We should also check if the value isn't negative.
In general, is there a reason not to use https://pkg.go.dev/net#SplitHostPort for parsing here, instead of the regex?
There was a problem hiding this comment.
this was copied over from the existing code, but i can swap it :)
There was a problem hiding this comment.
actually the problem with net.SplitHostPort is that it doesn't handle the case where it contains a scheme at the beginning (obv not a major issue) but that probably is why it wasn't used initially. The code ends up being the same here actually, so I think I may stick with this just changing to a 16 bit int... actually i think we need to use 32 bits here because otherwise we get an out of range error:
strconv.ParseInt: parsing "65535": value out of range
(ex)
Description:
This is the first part of a multi-PR effort to simplify how parsing configuration blocks works. This is accomplished by massively reducing the toil of creating new parsers and limiting the state objects need to hold on to. Right now, none of these new components are being used, as that will be isolated for a follow up.
Link to tracking Issue(s): #2603
Testing: many unit tests
Documentation: n/a