-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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/aws: Add S3 Bucket Object (supercedes #2079) #2898
Conversation
* master: (720 commits) Update CHANGELOG.md Update CHANGELOG.md dynamodb-local Update AWS config #2825 (comment) Make target_pools optional Update CHANGELOG.md code formatting Update CHANGELOG.md providers/google: Fix reading account_file path providers/google: Fix error appending providers/google: Return if we could parse JSON providers/google: Change account_file to JSON providers/google: Default account_file* to empty providers/google: Add account_file/account_file_contents ConflictsWith providers/google: Document account_file_contents providers/google: Use account_file_contents if provided providers/google: Add account_file_contents to provider Update CHANGELOG.md Update CHANGELOG.md dynamodb-local Use ` instead of : to refer region to keep the consistency with the provider docs dynamodb-local Update aws provider docs to include the `dynamodb_endpoint` argument ...
d.SetId("") | ||
log.Printf("Error Reading Object (%s): %s", key, err) | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems perhaps a little too wide of a net for the error. Like if I have a wifi hiccup I don't want TF to assume objects are gone. Can we scope this down to an "Object Not Found" error and return anything else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tried to scope this, but with no luck:
log.Printf("\n\n--- Error: %#v\n------\n", err)
if awsErr, ok := err.(awserr.Error); ok {
log.Printf("\n\n-----\n AWS ERror here:\n\tCode: %s\n\tMessage: %s", awsErr.Code(), awsErr.Message())
log.Printf("\n\n-----\n main error: %s", err.Error())
}
Response:
: --- Error: &awserr.requestError{awsError:(*awserr.baseError)(0xc208348b10), statusCode:404, requestID:""}
: ------
: 2015/08/03 10:06:44
:
: -----
: AWS ERror here:
: Code:
: Message:
😱
@phinze I did check GetObject
(instead of HeadObject
), which does return NoSuchKey
for the code. I'm not sure if we should switch though, because of the included network traffic for potentially larger objects. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@catsby from that output it looks like something like this would work for a scope:
if awsErr, ok := err.(awserr.RequestError); ok && awsErr.StatusCode == 404 {
// ...
}
Similar to how we do it for S3 Buckets. err.(awserr.RequestFailure)
might work too, which would make us able to use literally the same snippet.
Let me know what you think.
…r-2079 * upstream/master: Update CHANGELOG.md Update CHANGELOG.md provider/aws: allow external ENI attachments Update AWS provider documentation docs/aws: Fix example of aws_iam_role_policy provider/aws: S3 bucket test that should fail provider/aws: Return if Bucket not found Update CHANGELOG.md Update CHANGELOG.md helper/schema: record schema version when destroy fails settings file is not required provider/azure: Allow settings_file to accept XML string add note to aws_iam_policy_attachment explaining its use/limitations docs: clarify template_file path information google: Sort resources by alphabet in docs Support go get in go 1.5 Update CHANGELOG.md aws_network_interface attachment block is not required provider/aws: Fix issue in Security Group Rules where the Security Group is not found
7cefd53
to
9abd59a
Compare
9abd59a
to
bfaea76
Compare
provider/aws: Add S3 Bucket Object (supercedes #2079)
website: adding production hardening guide
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. |
Supersedes #2079:
master
(as of 6e7c612)key
instead of the returnedetag
. The reason here is that theetag
is typically the md5 of the object, but not if you supply your own encryption keys for the object. While we don't support that yet, I don't want to add it in the future and then mess up existing objectsS3 Bucket Object
to the docs sidebaretag
awslabs
foraws