Skip to content
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

Implement Additional ELB Connection Attributes #1502

Merged
merged 1 commit into from
Apr 22, 2015
Merged

Implement Additional ELB Connection Attributes #1502

merged 1 commit into from
Apr 22, 2015

Conversation

jwaldrip
Copy link
Contributor

No description provided.

d.SetPartial("connection_draining")
d.SetPartial("connection_draining_timeout")
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modifying both idle_timeout or connection_draining (and/or partner connection_draining_timeout) results in two ModifyLoadBalancerAttributes requests to AWS, but it looks like the API should support both of these in one request. Do you know if that's accurate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure on how to handle this, cross_zone_load_balancing does the same. I could do it all in one request if thats preferable. I order to do so I would just see if any of the ELB attributes supported keys are changed and then compose the call. Let me know if you want me to go that route and I will make the change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose it depends on how AWS handles this on their side. I know for Route 53 for example you can get errors returned if you make a modification request while another modification request is still in-flight. If ELB are quicker, or don't do that at all, then it's just an optimization nice to have. But if for example changing both results in the latter failing, then obviously we need to avoid that. I'm OK leaving as is, assuming that is not the case.

@catsby
Copy link
Contributor

catsby commented Apr 13, 2015

Thanks for the contribution, very nice! Just a few questions for you, posted in-line

@@ -59,6 +62,9 @@ The following arguments are supported:
* `listener` - (Required) A list of listener blocks. Listeners documented below.
* `health_check` - (Optional) A health_check block. Health Check documented below.
* `cross_zone_load_balancing` - (Optional) Enable cross-zone load balancing.
* `idle_timeout` - (Optional) Adjust Idle Timeout.
* `connection_draining` - (Optional) Enable connection draining.
* `connection_draining_timeout` - (Optional) Adjust the timeout for connection draining.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be helpful to be more descriptive here.

  • idle_timeout: The time in seconds that the connection is allowed to be idle. Default: whatever the default is
  • connection_draining - (Optional) Boolean to enable connection draining.
  • connection_draining_timeout - (Optional) The time in seconds to allow for connection draining. (or whatever)..

@catsby
Copy link
Contributor

catsby commented Apr 13, 2015

A few more follow ups, otherwise 👍

@jwaldrip
Copy link
Contributor Author

@catsby should be good to go.

@mitchellh
Copy link
Contributor

Thanks for answering our questions. This looks really good. And thanks a ton for the acceptance tests. That helps us verify this very easily.

@mitchellh mitchellh merged commit 74bfbec into hashicorp:master Apr 22, 2015
@ghost
Copy link

ghost commented May 3, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators May 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants