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

Allow string dates to stay strings in cache decoding #2695

Merged
merged 2 commits into from
Feb 24, 2023
Merged

Conversation

sanders41
Copy link
Contributor

Closes #2691

Code Changes

  • Prefix date objects when caching
  • Look for date prefix before decoding back to a date object

Steps to Confirm

  • Send values to cache that are a date object
  • Retrieve the values and verify they are returned as date objects
  • Send date values to cache formatted as a string
  • Retrieve the values and verify they are in string format

Pre-Merge Checklist

Description Of Changes

Write some things here about the changes and any potential caveats

@sanders41
Copy link
Contributor Author

@galvana will you see if this fixes your issue?

@cypress
Copy link

cypress bot commented Feb 24, 2023

Passing run #376 ↗︎

0 3 0 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge 850c742 into 72b5dfe...
Project: fides Commit: 8576c10ca9 ℹ️
Status: Passed Duration: 00:52 💡
Started: Feb 24, 2023 12:36 PM Ended: Feb 24, 2023 12:37 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 Feb 24, 2023

Codecov Report

Base: 86.46% // Head: 86.46% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (850c742) compared to base (72b5dfe).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2695      +/-   ##
==========================================
- Coverage   86.46%   86.46%   -0.01%     
==========================================
  Files         289      289              
  Lines       15984    15982       -2     
  Branches     2026     2027       +1     
==========================================
- Hits        13821    13819       -2     
  Misses       1779     1779              
  Partials      384      384              
Impacted Files Coverage Δ
src/fides/api/ops/util/cache.py 93.80% <100.00%> (-0.11%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sanders41 sanders41 changed the title Allow string dates to stay strings Allow string dates to stay strings in cache decoding Feb 24, 2023
@galvana
Copy link
Contributor

galvana commented Feb 24, 2023

Thanks @sanders41 that was it!

Copy link
Contributor

@galvana galvana left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for the quick turnaround on this

@sanders41 sanders41 merged commit e5ccd91 into main Feb 24, 2023
@sanders41 sanders41 deleted the ps-cache-date branch February 24, 2023 16:53
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.

Date strings are incorrectly returned as datetime objects after being stored and retrieved from Redis cache
2 participants