Skip to content
This repository has been archived by the owner on Aug 16, 2022. It is now read-only.

feat: Added glue table indexes #1377

Merged
merged 11 commits into from
Aug 11, 2022
Merged

Conversation

amanenk
Copy link
Contributor

@amanenk amanenk commented Aug 3, 2022

Summary

closes #1368
closes #1366


Use the following steps to ensure your PR is ready to be reviewed

  • Read the contribution guidelines πŸ§‘β€πŸŽ“
  • Run go fmt to format your code πŸ–Š
  • Lint your changes via golangci-lint run 🚨 (install golangci-lint here)
  • Update or add tests. Learn more about testing here πŸ§ͺ
  • Update the docs by running go run ./docs/docs.go and committing the changes πŸ“ƒ
  • If adding a new resource, add relevant Terraform files in a separate PR πŸ“‚
  • Ensure the status checks below are successful βœ…

@amanenk amanenk marked this pull request as ready for review August 3, 2022 08:03
@amanenk amanenk requested a review from a team as a code owner August 3, 2022 08:03
@disq
Copy link
Member

disq commented Aug 3, 2022

Maybe add a unit test just for the resolver? The documentation (godoc) for the method needs more clarification or examples as well.

docs/tables/aws_glue_database_table_partitions.md Outdated Show resolved Hide resolved
docs/tables/aws_glue_database_table_partitions.md Outdated Show resolved Hide resolved
docs/tables/aws_glue_database_table_partitions.md Outdated Show resolved Hide resolved
@bbernays
Copy link
Contributor

bbernays commented Aug 5, 2022

I like the resolver you added in this PR... My question looking at these fields is whether this is going to fetch too much data... as all of the nested calls are going to happen sequentially.... And looking at the Index/Partition data it doesn't seem that compliance/ security focused...

# Conflicts:
#	client/mocks/glue.go
#	client/services.go
#	resources/services/glue/databases.go
@amanenk amanenk changed the title feat: Added glue table partitions and indexes feat: Added glue table indexes Aug 9, 2022
@amanenk
Copy link
Contributor Author

amanenk commented Aug 9, 2022

I like the resolver you added in this PR... My question looking at these fields is whether this is going to fetch too much data... as all of the nested calls are going to happen sequentially.... And looking at the Index/Partition data it doesn't seem that compliance/ security focused...

@bbernays Agree on partitions. I have removed them. I have left indexes because it is infrastructural data that does not depend on data that is "fed" to glue

@amanenk amanenk merged commit b008f1b into cloudquery:main Aug 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AWS::Glue::Partition Add a resolver that transforms array of objects with two fields to json
5 participants