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

Added retry logic to retry when OpenStack returns 409 #26

Closed
wants to merge 1 commit into from

Conversation

slinn0
Copy link

@slinn0 slinn0 commented Jun 15, 2017

OpenStack will return 409 when creating multiple listeners at the same time. This patch will allow it to retry for 10 minutes when 409 or 500 is being returned.

Testing
Tested with the test case in hashicorp/terraform#15207
The listeners are now created as expected.

OpenStack will return 409 when creating multiple listeners
at the same time. This patch will allow it to retry for 10 minutes
when 409 or 500 is being returned.

**Testing**
Tested with the test case in hashicorp/terraform#15207
The listeners are now created as expected.
@fatmcgav
Copy link
Contributor

Potential fix for #4 and #23.

@fatmcgav fatmcgav requested a review from jtopjian June 15, 2017 06:05
@fatmcgav fatmcgav added the bug label Jun 15, 2017
@jtopjian
Copy link
Contributor

This looks to be the same solution that was implemented in hashicorp/terraform#15225, to which @jrperritt said he found to be incomplete.

@jrperritt
Copy link
Contributor

@slinn0 @fatmcgav it's unclear to me why y'all would press on with this after I said it was insufficient. Nevertheless, if there's interest, I have the proper fix already prepared and waiting on #25 to merge.

@fatmcgav
Copy link
Contributor

@jrperritt Apologies, must have missed that comment...

@slinn0
Copy link
Author

slinn0 commented Jun 15, 2017

@jrperritt It was not clear what was not sufficient. I know that the first PR was not complete because it was not tested or functioning. I had already made a patch against the external providers and tested that and verified that it worked for our test case so I made a PR that fixed our test case because it complete for our test cases.

@slinn0 slinn0 closed this Jun 15, 2017
@jtopjian
Copy link
Contributor

All, the fault lies with me for delaying any work on this. I'm trying to get #25 merged in as it touches every file in the provider and merging it now will prevent any conflicts for subsequent merges.

I apologize for any frustration or confusion here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants