-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
fix(permalink): migrate to marshmallow codec #24166
fix(permalink): migrate to marshmallow codec #24166
Conversation
{ | ||
"dashboardId": "1", | ||
"state": { | ||
"urlParams": [("foo", "bar"), ("foo", "baz")], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notice how the decoded value now contains tuples which aren't natively supported by JSON.
Codecov Report
@@ Coverage Diff @@
## master #24166 +/- ##
==========================================
- Coverage 68.27% 68.26% -0.01%
==========================================
Files 1952 1955 +3
Lines 75367 75451 +84
Branches 8208 8215 +7
==========================================
+ Hits 51455 51510 +55
- Misses 21806 21835 +29
Partials 2106 2106
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
d60b2b2
to
dd9720b
Compare
(cherry picked from commit 71d0543)
SUMMARY
A recent PR #23888 that migrated serialization of permalink entries from Pickle to JSON caused a regression when decoding permalinks containing URL params. More specifically, this happened due to the fact that we were encoding the value with a JSON codec, which doesn't distinguish between tuples and lists. As the
urlParams
property in the permalink state was of the formatOptional[List[Tuple[str, str]]]
, they becameOptional[List[List[str]]]
after deserialization, whichurllib.parse.urlencode
is unable to process.To get around this we introduce a new codec called
MarshmallowKeyValueCodec
. This extends theJsonKeyValueCodec
but takes a Marshmallow Schema in the constructor, and uses that during encoding/decoding. This ensures that an entry that has been serialized in JSON format can contain tuples after decoding.Thanks @Always-prog for raising this issue in #24160.
AFTER
Now permalinks containing
urlParams
work as expected:BEFORE
Previously permalinks with URL params would throw a 500:
TESTING INSTRUCTIONS
abc=123
ADDITIONAL INFORMATION