villebro commented on issue #9572: URL: https://github.com/apache/incubator-superset/pull/9572#issuecomment-618392655
> @villebro I took a brief stab at this ([commit](https://github.com/apache/incubator-superset/pull/9572/commits/145b8ea039468aae447a44ed632efdcb6fba5372)) though it's untested. > > You have more context here but my sense is that the `cache_key_wrapper` method may now be redundant. Maybe it would be best if you took over this PR (per your previous suggestion) as you're much more well versed on exactly how this all works. The `cache_key_wrapper` method is good to leave as-is for cases where users want to add values to `extra_cache_keys` other than the predefined `current_username()` etc. However, the documentation probably needs to be updated to reflect the fact that the predefined jinja functions don't need to be wrapped. > My one concern is it seems that the `extra_cache_keys` is merely a list of values, i.e., `user_1`, `user_2` etc. however there's not scope and thus there could be collision. I wonder if it should be a dictionary of values keyed by method with more context, i.e,. > > ```python > { > "current_user_id": ["user_1", "user_2",], > "url_param": [{"foo": "bar"},], > } > ``` > > Note in the case of the URL parameters having both the key and value is probably necessary for completeness. With regards to `extra_cache_keys` being a list, I don't think it's a big issue, as the extra cache keys will probably always be added sequentially. So if we're adding two keys `key1` and `key2`, with values `value1` and `value2` respectively, it probably won't make a big difference if we're using `{"key1": "value1", "key2": "value2"}` or `["value1", "value2"]`. While there is the theoretical risk that the code adds different keys inder different circumstances, to my understanding this has not been an issue during the time this has been available (since last summer). ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
