This repository has been archived by the owner on Dec 4, 2024. It is now read-only.
Allow LB to work for apps with IP per task #144
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Fix up LB to use correct IP address and port. For IP-per-task, use actual task IP and port.
| @@ -1,3 +1,4 @@ | |||
| .idea | |||
| templates/ | |||
| __pycache__ | |||
| *.pyc | |||
There was a problem hiding this comment.
This is a nit, but small whitespace error here (no newline at EOF).
|
Looks great overall. We just need a test around the new function, and we're good to go. |
|
@brndnmtthws I've updated the review with some UTs to cover all branches in that method. What's the take on d2iq-archive/marathon#3636 ? Any chance someone could fix that - I'm afraid that one is a little harder for me to get my head around. |
|
I'm not sure about the Marathon issue at this point, but I'll check in with the Marathon team to see if we can get some resources on it. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #139
I've added support for IP per task. There are currently no additional UTs here, but I have fixed up the existing ones.
Note that there is a marathon issue d2iq-archive/marathon#3636 that will prevent IP-per-tasks from working still, so this is only part of the overall fix required. Ideally it would be great if someone could get a fix in Marathon for this - I'm a little out of my depth to make that fix. Could you let me know if is likely to get fixed as it's blocking me. If not I can provide another PR that works around the missing data in the marathon-lb - but the fix is only useful in certain circumstances.