-
Notifications
You must be signed in to change notification settings - Fork 14k
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): remove memoize on get salt func #19749
Conversation
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.
LGTM
Codecov Report
@@ Coverage Diff @@
## master #19749 +/- ##
==========================================
- Coverage 66.51% 66.51% -0.01%
==========================================
Files 1687 1687
Lines 64620 64618 -2
Branches 6646 6646
==========================================
- Hits 42980 42978 -2
Misses 19940 19940
Partials 1700 1700
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
(cherry picked from commit cf51459)
🏷️ preset:2022.15 |
(cherry picked from commit cf51459)
SUMMARY
Currently the util function that fetches the permalink salt is memoized. This can cause trouble if the workers need to reference different metastores. While removal of the
memoized
decorator will have a performance impact, it should be negligible, as it's very light weight and occurs fairly infrequently. I will look into a better solution for avoiding refetching the salt on each permalink action.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Decoding a permalink with a salt different to that with which it was encrypted results in the following error:
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION