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

Conversation

galvana
Copy link
Collaborator

@galvana galvana commented Mar 21, 2022

Purpose

To fix the issue described in #308.

Changes

Modified the filter and unwrap post-processors to return an empty list [] for the following scenarios:

Filter

  • Empty data passed into filter
  • Identity data missing
  • Field not available on the object being filtered
  • No results remaining after filter (how this issue was originally discovered)

Unwrap

  • No data found at data_path

Checklist

  • Applicable documentation updated (guides, quickstart, postman collections, tutorial, fidesdemo, database diagram)
  • Good unit test/integration test coverage
  • This PR contains a DB migration. If checked, the reviewer should confirm with the author that the down_revision correctly references the previous migration before merging
  • The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

Ticket

Fixes #308

# flatten the list to account for the event where the output of unwrapped
# is a list of lists
unwrapped = pydash.flatten(unwrapped)

if unwrapped is None:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved this inside for loop to log a warning for each occurrence.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for restructuring, much more explicit per occurrence 💯

def process(self, data: Any, identity_data: Dict[str, Any] = None) -> Any:
def process(
self, data: Any, identity_data: Dict[str, Any] = None
) -> Union[List[Dict[str, Any]], Dict[str, Any]]:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Being more specific with return type since we know now to expect a dict or a list.

@seanpreston seanpreston added the run unsafe ci checks Triggers running of unsafe CI checks label Mar 21, 2022
Copy link
Contributor

@eastandwestwind eastandwestwind left a comment

Choose a reason for hiding this comment

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

lgtm, nice work!

@eastandwestwind eastandwestwind merged commit c54754c into main Mar 22, 2022
@eastandwestwind eastandwestwind deleted the 308-post-processors-returning-none-will-cause-the-retrieve_data-function-to-retry-until-failure branch March 22, 2022 16:36
sanders41 pushed a commit that referenced this pull request Sep 22, 2022
#309)

* Returning an empty list instead of None when post-processors filter all data or unwrap to an empty value

* Re-adding code missed during cleanup

* Fixing formatting

Co-authored-by: Adrian Galvan <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
run unsafe ci checks Triggers running of unsafe CI checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Post-processors returning 'None' will cause the retrieve_data function to retry until failure
3 participants