EC2 ELB listeners now support InstanceProtocol without breaking existing syntax #1048

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
6 participants
Contributor

marknca commented Oct 12, 2012

In response to open pull #562, I've implemented the InstanceProtocol in the ELB listeners without breaking the existing syntax.

This isn't the cleanest implementation if we were starting today but it is a solid solution that maintains backwards compatibility given the design choices that were made with the existing code.

When creating a load balancer or listener, the listeners argument is now optional. I've added a complex_listeners argument at the end of the function definition that handles a longer tuple.

The existing listeners argument is a list of tuples 3-4 items in length. The complex_listeners argument accepts a list of tuples 4-5 items in length. You can use both arguments at the same time but at least one is required.

This pull request now allows users to create listeners that are HTTPS <> HTTPS instead of the default HTTPS <> HTTP when HTTPS is specified as the protocol.

# existing listener tuple
(port, instance_port, protocol, ssl_id)

# complex listener tuple
(port, instance_port, protocol, instance_protocol, ssl_id)

Suggestions are welcome but this seemed the most elegant solution given the existing constraints.

Contributor

katzj commented Jan 3, 2013

Any update on this one?

hypexr commented Jan 24, 2013

Are there plans to merge this, implement an alternate solution, or should I fork to get this pull request? Thanks.

Contributor

marknca commented Jan 28, 2013

@katzj @hypexr I haven't heard anything from the team on this yet...

em-cliqz commented May 2, 2013

+1

Contributor

toastdriven commented May 2, 2013

@marknca Part of the reason it hadn't landed yet is that it's just complex enough that we'd like tests to ensure there aren't any regressions or whatnot. The other part has been a lack of time. I've started on some tests & once I have things passing, we'll try to get this landed. I'll update this PR once that happens.

Contributor

toastdriven commented Jul 9, 2013

Merged in SHA: b782ce2

toastdriven closed this Jul 9, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment