Enable use of hostname from nats links #530
Conversation
|
We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story. The labels on this github issue will be updated when the story is started. |
| nats_machines = ips.compact | ||
| end.else_if_link('nats') do | ||
| nats_machines = link('nats').instances.map { |instance| instance.address } | ||
| end.else do |
There was a problem hiding this comment.
I'm not sure if getting rid of the if_link('nats') check would break anything
There was a problem hiding this comment.
So this is not really an issue. The previous code if there was no nats link and no nats.machines specified would set nothing for the nats addresses. And the route_emitter would fail connecting to nats.
Now this code will, if there is no nats.machines specified and there is no nats link, fail trying to get the nats link on line 8.
Both will have the route_emitter failing. This code is exactly the same as what the route_registrar does and that is why I implemented it as such.
If you want the route_emitter to fail the same way it does now instead of failing this new way we could change line 7 to match the end.else_if_link('nats') do
But I do feel that it is better to fail during the deployment rather than have a successful deploy that when the route_emitter starts will not be able to connect to nats
There was a problem hiding this comment.
If I understand Andrew correctly, previously, if there was no nats link and no nats.machines specified, then the code would set nothing for the nats addresses. And then the route_emitter would subsequently fail at runtime because it would be unable to connect to nats as there were no machines/addresses.
This PR will change the behavior so that it will fail at deploy time because it cannot get the required nats link. That is better than failing at runtime
| nats_machines = ips.compact | ||
| end.else_if_link('nats') do | ||
| nats_machines = link('nats').instances.map { |instance| instance.address } | ||
| end.else do |
There was a problem hiding this comment.
same comment as above
|
the changes that were requested were done on further chatting they were required and it was simple. |
4 participants
Thank you for submitting a pull request to the diego-release repository. We appreciate the contribution. To help us with getting better context for the pull request please follow these guidelines:
Please make sure to complete the following steps
developbranch.Issue Link
#529
Thank you!