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

Additional Key Columns for aws_accessanalyzer_finding Table #2331

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dbermuehler
Copy link
Contributor

This PR adds the following key columns for the list function of the aws_accessanalyzer_finding table:

  • error
  • is_public
  • resource_owner_account
  • resource_type

Integration test logs

Logs
Add passing integration test logs here

Example query results

Results
Add example SQL query results here (please include the input queries as well)

@misraved misraved self-requested a review November 1, 2024 13:23
@misraved misraved added the hacktoberfest-accepted This pull request has been accepted for Hacktoberfest label Nov 1, 2024
Copy link
Contributor

@misraved misraved left a comment

Choose a reason for hiding this comment

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

@dbermuehler thank you so much for the PR 👍 !!,

While testing it out, did you see a significant time benefit in adding these columns?

Also, could you please add is_public, resource_owner_account and resource_type to the list of columns as well?

You can use

Transform: transform.FromQual("end_time"),
for additional references.

Comment on lines 222 to 227
var ae smithy.APIError
if errors.As(err, &ae) {
if ae.ErrorCode() == "ResourceNotFoundException" || ae.ErrorCode() == "ValidationException" {
return nil, nil
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@dbermuehler we need to handle these errors separately since we are using the ParentHydrate listAccessAnalyzers for this table.

SDK issue reference - turbot/steampipe-plugin-sdk#544

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see! I reverted the changes.

@dbermuehler
Copy link
Contributor Author

@dbermuehler thank you so much for the PR 👍 !!,

While testing it out, did you see a significant time benefit in adding these columns?

Also, could you please add is_public, resource_owner_account and resource_type to the list of columns as well?

You can use

Transform: transform.FromQual("end_time"),

for additional references.

These key columns exist already as columns for this table.

Regarding speed improvement: I have to admit the increase in speed was quite marginal. For the tests I did it made the queries 1 - 2 seconds faster. This however might be different for bigger data sets then the one I have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted This pull request has been accepted for Hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants