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

provider/Azure: various bugfixes #3695

Merged
merged 1 commit into from
Oct 30, 2015
Merged

Conversation

aznashwan
Copy link
Contributor

I cannot thank @keymon enough for the most absolutely outrageously amazing bug reports I've received in my life.

}

// unfortunately, there's no better way of doing this except
// polling continuously until it goes away:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we toss an outer limit timeout on this?

We have a resource.Retry helper that's commonly used elsewhere for this purpose:

https://github.com/hashicorp/terraform/blob/master/helper/resource/wait.go#L12-L36

Example usage:

https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/resource_aws_autoscaling_group.go#L374-L394

@phinze
Copy link
Contributor

phinze commented Oct 29, 2015

One comment - otherwise this LGTM!

@aznashwan
Copy link
Contributor Author

@phinze: I should really spend a day or two in the helper package; at least it'll preventing me from re-inventing wheels which are square compared to you guys' 😄

storageContainterName, fmt.Sprintf(osDiskBlobNameFormat, name),
)
if err != nil {
return err
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this error retryable? If you want the error to break the Retry loop immediately, wrap it in a resource.RetryError

added wait on instance deletion for associated blob deletion.
added guarding Mutex for secgroup-rule-related concurrent operations.
added usage warning on secgroup rules.
@aznashwan
Copy link
Contributor Author

@phinze: good catch on that! Updated and ready to go 😄

@phinze
Copy link
Contributor

phinze commented Oct 30, 2015

LGTM! Merging...

phinze added a commit that referenced this pull request Oct 30, 2015
provider/Azure: various bugfixes
@phinze phinze merged commit ee4033e into hashicorp:master Oct 30, 2015
@ghost
Copy link

ghost commented Apr 30, 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 Apr 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants