dpgaspar commented on PR #35621:
URL: https://github.com/apache/superset/pull/35621#issuecomment-3611676256
> > existing keys (for permalinks) are still discovereble
>
> What about application configs? It would be important to raise exactly
what type of information can't be re-generated to make sure fallbacks are
applied to all. Also, wouldn't be better to save the hash type in the database
(as a separate column or as a prefix for the hash like `md5$<hashvalue>`
instead of a try/error approach with fallbacks?
Regarding application configs: Yes, they're covered. The get_shared_value()
function in superset/key_value/shared_entries.py implements the same fallback
mechanism for app configs (like the
permalink salt). When found via fallback, it also auto-migrates the entry
to the current algorithm's UUID.
Regarding storing the hash type (column or prefix): I considered this
approach, but here's why I chose the fallback pattern instead:
1. The hash isn't stored directly - The hash algorithm generates a UUID
namespace, and that UUID is what's stored. To track the algorithm, we'd need a
new column on key_value entries, requiring a
DB migration + backfill.
2. Fallback scope is very limited - It's only used in two places:
- CreateDashboardPermalinkCommand: To check for duplicates before
creating (prevents creating a new permalink if one exists with the old
algorithm)
- get_shared_value(): For app configs like the permalink salt
Notably, GET permalink by URL doesn't need fallback - the URL contains an
encoded integer ID, so lookups are direct regardless of which algorithm created
it.
3. Trivial algorithm set - With only 2-3 algorithms (md5, sha256, maybe
sha512 someday), the "try-error" cost is one extra UUID generation + DB lookup
in the worst case - microseconds.
4. Transitional by nature - Fallback is only for legacy entries. New
entries use the current algorithm, so the fallback list naturally becomes less
relevant over time.
Given the limited scope (duplicate detection + app configs only) and the
migration complexity a column approach would require, the fallback pattern
seemed like the right trade-off. But happy to discuss further!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
