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

(enhancement): Extend get and update ruleset DQ methods #1882

Merged
merged 3 commits into from
Dec 19, 2022

Conversation

jaidisido
Copy link
Contributor

Feature or Bugfix

  • Enhancement

Detail

  • get_ruleset should accept multiple rulesets and combine them to a data frame
  • At the moment update_ruleset just overwrites the existing ruleset. Introducing a mode argument (overwrite, upsert) to handle upserts
  • Wrap get methods in try_it to handle request throttling
  • Add tests

Relates

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -134,6 +136,8 @@ def update_ruleset(
Ruleset name.
updated_name : str
New ruleset name if renaming an existing ruleset.
mode : str
overwrite (default) or upsert.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

overwrite and upsert are the only modes I can think of

Copy link
Contributor

Choose a reason for hiding this comment

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

How about append?

if mode not in ["overwrite", "upsert"]:
raise exceptions.InvalidArgumentValue("`mode` must be one of 'overwrite' or 'upsert'.")

if mode == "upsert":
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could not find a better way to do an upsert in pandas

for ruleset_name in ruleset_names:
rules = cast(str, _get_ruleset(ruleset_name=ruleset_name, boto3_session=boto3_session)["Ruleset"])
df = _rules_to_df(rules=rules)
if len(ruleset_names) > 1:
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 think a column with the ruleset name should only be added if there is multiple ones. But let me know if you disagree

Copy link
Contributor

Choose a reason for hiding this comment

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

What's the major downside to adding the ruleset name always? I like the idea of the structure of the DataFrame to be consistent (i.e. it should either always have ruleset).

for ruleset_name in ruleset_names:
rules = cast(str, _get_ruleset(ruleset_name=ruleset_name, boto3_session=boto3_session)["Ruleset"])
df = _rules_to_df(rules=rules)
if len(ruleset_names) > 1:
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the major downside to adding the ruleset name always? I like the idea of the structure of the DataFrame to be consistent (i.e. it should either always have ruleset).

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubCodeBuild8756EF16-4rfo0GHQ0u9a
  • Commit ID: 605be51
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubCodeBuild8756EF16-4rfo0GHQ0u9a
  • Commit ID: 82cd555
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubCodeBuild8756EF16-4rfo0GHQ0u9a
  • Commit ID: 516637a
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@jaidisido jaidisido merged commit 93696bb into main Dec 19, 2022
@jaidisido jaidisido deleted the enhancement/extend-dq-get-update branch December 19, 2022 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants