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/aws: aws_ami_description - data source edition (now aws_ami) #6911

Merged

Conversation

vancluever
Copy link
Contributor

This is #4396 in data source form. See that issue/PR for details.

Incidentally, porting this was very easy, with one small testing caveat - to test the "resource" properly, I had to prefix the name with data to ensure that it could be looked up in the state. This may just be semantics we have to live with, or could be the basis for a quality of life update.

I also ran into the same bug that @jen20 is running into on #6881, for which I have a fix incoming shortly.

@vancluever vancluever changed the title provider/aws: aws_ami_description - data source edition [WIP] provider/aws: aws_ami_description - data source edition May 28, 2016
@vancluever vancluever force-pushed the paybyphone_aws_ami_description_data_source branch 2 times, most recently from 5301288 to b90f999 Compare May 28, 2016 16:50
@vancluever vancluever changed the title [WIP] provider/aws: aws_ami_description - data source edition provider/aws: aws_ami_description - data source edition May 28, 2016
@vancluever
Copy link
Contributor Author

All done, just needed to fix the docs :)

vancluever pushed a commit to paybyphone/terraform that referenced this pull request May 28, 2016
During accpeptance tests of some of the first data sources (see
hashicorp#6881 and hashicorp#6911),
"unknown resource type" errors were coming up. It looks like
DataSourcesMap was not being taken into account in
*schema.Provider.Apply(), causing this error.

Also, added a skip in *schema.Resource.Apply() if
*schema.Resource.Destroy() is not defined, as data sources do not define
this, fixing the first error resulted in a nil pointer dereference.
vancluever pushed a commit to paybyphone/terraform that referenced this pull request May 28, 2016
During accpeptance tests of some of the first data sources (see
hashicorp#6881 and hashicorp#6911),
"unknown resource type" errors were coming up. It looks like
DataSourcesMap was not being taken into account in
*schema.Provider.Apply(), causing this error.

Also, added a skip in *schema.Resource.Apply() if
*schema.Resource.Destroy() is not defined, as data sources do not define
this, fixing the first error resulted in a nil pointer dereference.
vancluever pushed a commit to paybyphone/terraform that referenced this pull request May 29, 2016
During accpeptance tests of some of the first data sources (see
hashicorp#6881 and hashicorp#6911),
"unknown resource type" errors have been coming up. Traced it down to
the ResourceCountTransformer, which transforms destroy nodes to a
graphNodeExpandedResourceDestroy node. This node's EvalTree() was still
indiscriminately using EvalApply for all resource types, versus
EvalReadDataApply. This accounts for both cases via EvalIf.
This data source allows one to look up the most recent AMI for a specific
set of parameters, much like aws ec2 describe-images in the AWS CLI.

Basically a refresh of hashicorp#4396, in data source form.
@vancluever vancluever force-pushed the paybyphone_aws_ami_description_data_source branch from b90f999 to 9ac7fb0 Compare May 29, 2016 16:56
@vancluever vancluever changed the title provider/aws: aws_ami_description - data source edition provider/aws: aws_ami_description - data source edition (now aws_ami) May 29, 2016
@vancluever
Copy link
Contributor Author

I've renamed this to aws_ami since we can have overlapping resource and data source names.

@jen20
Copy link
Contributor

jen20 commented May 29, 2016

make testacc TEST=./builtin/providers/aws TESTARGS="-run TestAccAWSAmiDataSource"
==> Checking that code complies with gofmt requirements...
/Users/James/Code/go/bin/stringer
go generate $(go list ./... | grep -v /vendor/)
2016/05/29 12:04:27 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run TestAccAWSAmiDataSource -timeout 120m
=== RUN   TestAccAWSAmiDataSource_natInstance
--- PASS: TestAccAWSAmiDataSource_natInstance (11.25s)
=== RUN   TestAccAWSAmiDataSource_windowsInstance
--- PASS: TestAccAWSAmiDataSource_windowsInstance (13.57s)
=== RUN   TestAccAWSAmiDataSource_instanceStore
--- PASS: TestAccAWSAmiDataSource_instanceStore (10.34s)
=== RUN   TestAccAWSAmiDataSource_owners
--- PASS: TestAccAWSAmiDataSource_owners (30.14s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    65.330s

Great work @vancluever!

@jen20 jen20 merged commit 5cb38c5 into hashicorp:master May 29, 2016
@vancluever
Copy link
Contributor Author

Woohoo! Thanks @jen20!

@vancluever vancluever deleted the paybyphone_aws_ami_description_data_source branch May 30, 2016 01:48
@ghost
Copy link

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

Successfully merging this pull request may close these issues.

3 participants