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

New resource - Glue crawlers #4484

Closed
wants to merge 60 commits into from

Conversation

darrenhaken
Copy link
Contributor

@darrenhaken darrenhaken commented May 8, 2018

Closes #3875

Changes proposed in this pull request:

  • Add support for a new resource, Glue Crawlers.

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSGlueCrawler'

Move back to creating an acceptance test for the required fields on a
crawler.

Having issues working out how to declare the schema for the Targets in
the Resource Schema.

Failing acceptance test claims its something to do with the syntax of
the test.
@ghost ghost added the size/XL Managed by automation to categorize the size of a PR. label May 8, 2018
@bflad bflad added new-resource Introduces a new resource. service/glue Issues and PRs that pertain to the glue service. labels May 9, 2018
@darrenhaken
Copy link
Contributor Author

@bflad I am still having issues having this work for me.

If you run the test do you also see failures around IAM and assuming the role?
I have been stumped by this and it has meant I haven't really come back to the PR to carry on with it.

Could you help fix this with me and I'll carry on with the rest of the PR?

@darrenhaken
Copy link
Contributor Author

@bflad still not heard back, do you think it's something you can look at?

We're using the Crawlers quite a lot now and getting it integrated into TF would be fantastic.

@f0rk
Copy link
Contributor

f0rk commented May 30, 2018

@darrenhaken I have resolved the role issue, you can see it here: darrenhaken#1

@darrenhaken
Copy link
Contributor Author

@f0rk awesome! I'll give this a try tomorrow and confirm if it works. I appreciate the help.

make glue crawler creation wait for role ready
@ghost ghost added the size/XL Managed by automation to categorize the size of a PR. label May 31, 2018
@darrenhaken
Copy link
Contributor Author

Looks like it's working @f0rk and I've included a few more of the fields to the crawler.

@ghost ghost added the size/XL Managed by automation to categorize the size of a PR. label Jun 1, 2018
@jordanfowler
Copy link

How's this looking? We'd love to get this into our terraform stack.

@jordanfowler
Copy link

@mabushey 🌮

@darrenhaken
Copy link
Contributor Author

@jordanfowler I've been on vacation but I'm picking this back up atm. I have the basic Crawler working and now I'm getting through all the optional fields.

You're welcome to work on it as well if you'd like.

I'm making good progress on it now though.

- Remove classifiers for now as seems to be causing issues. Will look
into it later
@ghost ghost added size/XXL Managed by automation to categorize the size of a PR. and removed size/XL Managed by automation to categorize the size of a PR. labels Jun 20, 2018
@darrenhaken
Copy link
Contributor Author

@bflad thanks for getting back to me with the changes.

I'll do my best to change what I can today but if you can finish some of it off I'd certainly appreciate it. I'm quite swamped this week 👎

I have already pushed the rename and I'll do see what I can do on the rest.

@ghost ghost added the size/XXL Managed by automation to categorize the size of a PR. label Jun 20, 2018
@darrenhaken
Copy link
Contributor Author

@bflad I have made a commit which fixes most of the issues you've raised. Do you think you could do the last few? I ran out of time.

@bflad
Copy link
Contributor

bflad commented Jun 20, 2018

@darrenhaken you rock 🥇 I'll add one last commit after yours to get this across the finish line. We should be able to release this today! More soon.

@bflad bflad added this to the v1.24.0 milestone Jun 20, 2018
@darrenhaken
Copy link
Contributor Author

darrenhaken commented Jun 20, 2018 via email

@ghost ghost added the size/XXL Managed by automation to categorize the size of a PR. label Jun 20, 2018
@darrenhaken
Copy link
Contributor Author

@bflad Sadly I had to revert my commit on the random acceptance test names. The basic test failed with a diff error and I do not have any time left today to take a look.

If you could finish the last bits off I'd really appreciate it. We will be really pleased when this goes in :)

Do you think you can?

P.S I didnt get chance to do the docs.

This caused the basic acceptance test to fail for Glue Crawler
@darrenhaken
Copy link
Contributor Author

darrenhaken commented Jun 20, 2018

@bflad sorry to mess you around with this but I did some more investigation.

The basic test was failing by adding in one of your suggested changes:

if err := d.Set("schema_change_policy", []map[string]string{schemaPolicy}); err != nil {
			return fmt.Errorf("error setting schema_change_policy: %s", schemaPolicy)
		}

I have commented this out, for now, see line 342.

The plus is that I have been able to keep the random AC test names in.

@ghost ghost added the size/XXL Managed by automation to categorize the size of a PR. label Jun 20, 2018
@darrenhaken
Copy link
Contributor Author

@bflad any luck?

@bflad
Copy link
Contributor

bflad commented Jun 20, 2018

I have everything working in a commit after e5832d7:

make testacc TEST=./aws TESTARGS='-run=TestAccAWSGlueCrawler'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSGlueCrawler -timeout 120m
=== RUN   TestAccAWSGlueCrawler_JdbcTarget
--- PASS: TestAccAWSGlueCrawler_JdbcTarget (37.59s)
=== RUN   TestAccAWSGlueCrawler_JdbcTarget_Exclusions
--- PASS: TestAccAWSGlueCrawler_JdbcTarget_Exclusions (37.92s)
=== RUN   TestAccAWSGlueCrawler_JdbcTarget_Multiple
--- PASS: TestAccAWSGlueCrawler_JdbcTarget_Multiple (49.36s)
=== RUN   TestAccAWSGlueCrawler_S3Target
--- PASS: TestAccAWSGlueCrawler_S3Target (36.53s)
=== RUN   TestAccAWSGlueCrawler_S3Target_Exclusions
--- PASS: TestAccAWSGlueCrawler_S3Target_Exclusions (36.85s)
=== RUN   TestAccAWSGlueCrawler_S3Target_Multiple
--- PASS: TestAccAWSGlueCrawler_S3Target_Multiple (47.93s)
=== RUN   TestAccAWSGlueCrawler_Classifiers
--- PASS: TestAccAWSGlueCrawler_Classifiers (45.37s)
=== RUN   TestAccAWSGlueCrawler_Configuration
--- PASS: TestAccAWSGlueCrawler_Configuration (38.19s)
=== RUN   TestAccAWSGlueCrawler_Description
--- PASS: TestAccAWSGlueCrawler_Description (37.96s)
=== RUN   TestAccAWSGlueCrawler_Schedule
--- PASS: TestAccAWSGlueCrawler_Schedule (37.97s)
=== RUN   TestAccAWSGlueCrawler_SchemaChangePolicy
--- PASS: TestAccAWSGlueCrawler_SchemaChangePolicy (37.68s)
=== RUN   TestAccAWSGlueCrawler_TablePrefix
--- PASS: TestAccAWSGlueCrawler_TablePrefix (37.41s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	480.799s

bflad added a commit that referenced this pull request Jun 20, 2018
make testacc TEST=./aws TESTARGS='-run=TestAccAWSGlueCrawler'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSGlueCrawler -timeout 120m
=== RUN   TestAccAWSGlueCrawler_JdbcTarget
--- PASS: TestAccAWSGlueCrawler_JdbcTarget (37.59s)
=== RUN   TestAccAWSGlueCrawler_JdbcTarget_Exclusions
--- PASS: TestAccAWSGlueCrawler_JdbcTarget_Exclusions (37.92s)
=== RUN   TestAccAWSGlueCrawler_JdbcTarget_Multiple
--- PASS: TestAccAWSGlueCrawler_JdbcTarget_Multiple (49.36s)
=== RUN   TestAccAWSGlueCrawler_S3Target
--- PASS: TestAccAWSGlueCrawler_S3Target (36.53s)
=== RUN   TestAccAWSGlueCrawler_S3Target_Exclusions
--- PASS: TestAccAWSGlueCrawler_S3Target_Exclusions (36.85s)
=== RUN   TestAccAWSGlueCrawler_S3Target_Multiple
--- PASS: TestAccAWSGlueCrawler_S3Target_Multiple (47.93s)
=== RUN   TestAccAWSGlueCrawler_Classifiers
--- PASS: TestAccAWSGlueCrawler_Classifiers (45.37s)
=== RUN   TestAccAWSGlueCrawler_Configuration
--- PASS: TestAccAWSGlueCrawler_Configuration (38.19s)
=== RUN   TestAccAWSGlueCrawler_Description
--- PASS: TestAccAWSGlueCrawler_Description (37.96s)
=== RUN   TestAccAWSGlueCrawler_Schedule
--- PASS: TestAccAWSGlueCrawler_Schedule (37.97s)
=== RUN   TestAccAWSGlueCrawler_SchemaChangePolicy
--- PASS: TestAccAWSGlueCrawler_SchemaChangePolicy (37.68s)
=== RUN   TestAccAWSGlueCrawler_TablePrefix
--- PASS: TestAccAWSGlueCrawler_TablePrefix (37.41s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	480.799s
bflad added a commit that referenced this pull request Jun 20, 2018
@bflad
Copy link
Contributor

bflad commented Jun 20, 2018

Thanks so much for your work here @darrenhaken! All commits through e5832d7 are merged (sorry thought you were done pushing already earlier) and 6f41ed5 finishes the implementation including testing all the attributes individually with updates and documentation. I'll need to manually close this PR since GitHub will notice the last two commits in this branch missing from earlier today.

@bflad bflad closed this Jun 20, 2018
@bflad
Copy link
Contributor

bflad commented Jun 25, 2018

This has been released in version 1.24.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

@ghost
Copy link

ghost commented Apr 5, 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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new-resource Introduces a new resource. service/glue Issues and PRs that pertain to the glue service. size/XXL Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Resource for managing AWS Glue Crawlers
5 participants