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

feat: Updated code version of AWS promote quarantine plugin #123

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

Conversation

Anon4Now
Copy link

@Anon4Now Anon4Now commented Mar 9, 2023

Code changes to abide by Python best-practices

Change Summary

  • Refactored the code to meet more modern Object Oriented Programming techniques
  • Provide full write-up of changes below in notes section
  • Added the ability to tag an object in place, instead of it being skipped if no destination bucket is available

PR Checklist

  • [:ballot_box_with_check:] I've read and followed the Contributing Guide.
  • Documents/Readmes
    • Updated accordingly
    • [:ballot_box_with_check:] Not required
  • Plugins that have versioning
  • [:ballot_box_with_check:] Version bumped and follows Semantic Versioning
    • Not required

Other Notes

HIGH-LEVEL UPDATES

Lines 6-7: Imported dataclass function, field function, and types for annotation

Lines 72-82: Added type annotations to the get_mode_from_env function and added a docstring

Lines 85-106: Created a dataclass to take incoming event data and parse into instance attrs

Lines 109-293: Created a standard class that contains all attrs/methods that perform actions on S3 objects

Lines 296-330: Updated the Lambda Handler code and added docstring to function

BREAKDOWN OF EXISTING CODE

  1. All functions do not contain docstrings explaining purpose
  2. All functions do not contain type annotations to describe input and output
  3. All functions difficult to unit test in their current state
  4. All spacing is not PEP 8 compliant
  5. Functions get_promote_mode & get_quarantine_mode are unesscesary, as the default behavior of os.get.environ is None
  6. Function get_existing_tag_set:
    • Makes use of a try block and adds more than the AWS API call in the try
    • Does not make use of try/except/else for better segregation of code function
    • Redundant return statement as part of the except block
    • Does not make use of f-strings for print statement
  7. Function get_metadata:
    • Makes use of a try block and adds more than the AWS API call in the try
    • Does not make use of try/except/else for better segregation of code function
    • Redundant return statement as part of the except block
    • Does not make use of f-strings for print statement
  8. Function lambda_handler:
    • Far too complicated, needs code pulled out of body
    • No comments or internal documentation to denote different steps
    • Does not make use of f-strings for all print statements
    • Not scalable or easily customizable
    • Does not handle scenario where user does not want to use promote_bucket and just tag the object in place

BREAKDOWN OF NEW CODE

  1. All methods/functions contain docstrings that explain their intention
  2. All appropriate methods/functions/variables/attributes have type hints for better documentation
  3. Dataclass ObjectData:
    • Designed to parse incoming JSON string from SNS trigger in an efficient, scalable, and readable way
    • Modification for the parsing will only exist in one location if data being sent is ever altered
    • The post_init method parses the Python data structure param and assigns values to attributes
  4. Class AssignTag:
    • Takes the dataclass instance as an paramater to make use of dependency injection for testing
    • Takes previous functions and converts them to methods
    • Reduces actions for methods to a single purpose to abide by the Single Responsibility Principle
    • Uses dependency injection where possible to streamline unit test mocks
    • Private methods with the exception of the run method which serves an the public interface
  5. Method _parse_s3_object:
    • Assigned the src_bucket and object_key values to instance attributes for instance use
  6. Method _create_tags:
    • Moved ternary expressions related to tag building inside of related method
    • Assigned the created tag_list value to an instance attribute for general instance use
  7. Method _get_metadata:
    • Implemented try/except/else blocks for the AWS API call
    • Assigned the metadata value to an instance attribute for general instance use
  8. Method _get_existing_tag_set:
    • Implemented try/except/else blocks for the AWS API call
  9. Method _copy_object:
    • Implemented try/except blocks for the AWS API call
  10. Method _tag_object:
    • Net new method that is designed to handle situations where the destination bucket is not defined
    • Will parse the instance attribute tag_list and add that to the object in the source bucket
    • Implemented try/except blocks for the AWS API call
  11. Method _delete_object:
    • Removed bucket parameter from method as that data exists as an instance attribute
    • Implemented try/except blocks for the AWS API call
  12. Method run
    • Public method that will orchestrate the method calls in the correct order
    • Implements the conditional check for existing_tag_set using an assignment operator
    • Checks to see if a destination bucket was not passed as a paramater from the lambda_handler
  13. Function lambda_handler:
    • Reduced logic in lambda_handler
    • Removed function calls to get_promote_mode & get_quarantine_mode as those are redundant

@trend-jack-c-tang trend-jack-c-tang added the enhancement New feature or request label Mar 9, 2023
@trend-jack-c-tang trend-jack-c-tang requested a review from a team March 9, 2023 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants