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

Add additional error handling for fides_string override options #4325

Merged
merged 2 commits into from
Oct 23, 2023

Conversation

NevilleS
Copy link
Contributor

@NevilleS NevilleS commented Oct 23, 2023

Description Of Changes

Today, providing a fides_string with an AC suffix throws a runtime error when decoding, which is just because #4308 is in review. In the short-term, I'd like to get the current version "forwards-compatible" with that solution by just discarding the AC suffix. Ideally I should have included this in #4314 👍

Code Changes

  • Update TC string decoding to discard an AC suffix

Steps to Confirm

  • Run all automated Cypress tests (see changes!)

Pre-Merge Checklist

@cypress
Copy link

cypress bot commented Oct 23, 2023

Passing run #4761 ↗︎

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

Details:

Merge 1df0648 into 38f823b...
Project: fides Commit: 923f5b1eba ℹ️
Status: Passed Duration: 01:08 💡
Started: Oct 23, 2023 9:44 PM Ended: Oct 23, 2023 9:45 PM

Review all test suite changes for PR #4325 ↗︎

@NevilleS NevilleS changed the title Add a temporary solution for AC decoding for testing Add additional error handling for fides_string override options Oct 23, 2023
@NevilleS NevilleS marked this pull request as ready for review October 23, 2023 21:26
@NevilleS NevilleS merged commit 486d2fd into main Oct 23, 2023
9 of 10 checks passed
@NevilleS NevilleS deleted the ns-update-fides-string-decode branch October 23, 2023 21:52
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.

2 participants