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

Adding new scenario to refine_target_path #3906

Merged

Conversation

galvana
Copy link
Contributor

@galvana galvana commented Aug 7, 2023

Closes #3905

Description Of Changes

Updating refine_target_path to return an empty path if the path is invalid, meaning the data doesn't exist at the level indicated.

Code Changes

  • Added None value scenario to refine_target_path function

Steps to Confirm

  • Added new test cases to test_refine_target_path.py

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Issue Requirements are Met
  • Update CHANGELOG.md

@galvana galvana linked an issue Aug 7, 2023 that may be closed by this pull request
@galvana galvana requested a review from pattisdr August 7, 2023 21:53
@cypress
Copy link

cypress bot commented Aug 7, 2023

Passing run #3511 ↗︎

0 4 0 0 Flakiness 0

Details:

Merge c232ee7 into d97d2ad...
Project: fides Commit: cb0f9cbbbb ℹ️
Status: Passed Duration: 00:56 💡
Started: Aug 8, 2023 10:11 PM Ended: Aug 8, 2023 10:12 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@codecov
Copy link

codecov bot commented Aug 7, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (d97d2ad) 87.32% compared to head (c232ee7) 87.32%.

❗ Current head c232ee7 differs from pull request most recent head 4b92938. Consider uploading reports for the commit 4b92938 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3906   +/-   ##
=======================================
  Coverage   87.32%   87.32%           
=======================================
  Files         317      317           
  Lines       19397    19397           
  Branches     2494     2494           
=======================================
  Hits        16938    16938           
  Misses       2027     2027           
  Partials      432      432           
Files Changed Coverage Δ
src/fides/api/task/refine_target_path.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@pattisdr pattisdr left a comment

Choose a reason for hiding this comment

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

ah great improvement here - missed edge case, glad it's being addressed!

src/fides/api/task/refine_target_path.py Outdated Show resolved Hide resolved
@galvana galvana requested a review from pattisdr August 8, 2023 21:58
@galvana galvana marked this pull request as ready for review August 8, 2023 22:00
@galvana galvana merged commit 58d3388 into main Aug 8, 2023
11 of 12 checks passed
@galvana galvana deleted the 3905-null-object-incorrectly-overwritten-with-masked-value branch August 8, 2023 23:06
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.

Null object incorrectly overwritten with masked value
2 participants