-
Notifications
You must be signed in to change notification settings - Fork 37
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
Add endpoint to look up services by host and port #290
Conversation
throw Throwables.propagate(e); | ||
} | ||
return Optional.absent(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems horribly inefficient, but can't think of anything better since zk is backing it. Unless there's another place this data is stored I don't know about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check out how the state resource is grabbing this data. There should be a precompiled Collection (which includes the upstreams) in memory already
for (BaragonServiceState state : services) { | ||
for (UpstreamInfo upstreamInfo : state.getUpstreams()) { | ||
if (upstreamInfo.getUpstream().matches(String.format("%s\\.\\w+:%s", host, port))) { | ||
return state; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible that there is more than one, we should probably return a List/Collection
Collection<BaragonServiceState> services = objectMapper.readValue(stateCache.getState().getUncompressed(), new TypeReference<Collection<BaragonServiceState>>(){}); | ||
for (BaragonServiceState state : services) { | ||
for (UpstreamInfo upstreamInfo : state.getUpstreams()) { | ||
if (upstreamInfo.getUpstream().matches(String.format("%s\\.\\w+:%s", host, port))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the regex here instead of a straight .equals
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming people will send in params like adjective-noun
and 1234
, but the upstreams are in the format adjective-noun.env.network.net:1234
, so equals won't work. We can change the input type, or make this regex much more specific. Either is good with me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can expect the full dns name:port, the short hostnames is only an internal shortcut
No description provided.