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

4.1.1 migration troubleshooting with url_param() current_username() keywords in sql dataset #31975

Open
2 of 3 tasks
ERGO1995 opened this issue Jan 23, 2025 · 6 comments
Open
2 of 3 tasks
Assignees
Labels
global:async-query Related to Async Queries feature global:jinja Related to Jinja templating

Comments

@ERGO1995
Copy link

ERGO1995 commented Jan 23, 2025

Bug description

Hello, for the past three days we have been trying to migrate our Superset version from 4.0.2 to 4.1.1, but we are encountering an issue seemingly related to cache key retrieval. We have a source dataset behind our charts that uses keywords such as current_username(), cache_key_wrapper(), or url_param(). This issue only occurs when the GLOBAL_ASYNC_QUERIES flag is set to True.

Everything was working fine in 4.0.2, but now we are facing errors like this:

superset_app          | 2025-01-22 10:19:45,263:WARNING:superset.common.utils.query_cache_manager:force_cached (QueryContext): value not found for key 2791d577d99b83535ae58bcf40585cd1
superset_app          | 2025-01-22 10:19:45,349:WARNING:superset.common.utils.query_cache_manager:force_cached (QueryContext): value not found for key 2cdbfbda9ad8397e212070804634df6b
superset_worker       | [2025-01-22 10:19:45,390: INFO/MainProcess] Task load_chart_data_into_cache[2db1838d-7f84-44a2-8a8f-d3dc9db1d720] received
superset_worker       | [2025-01-22 10:19:45,429: INFO/MainProcess] Task load_chart_data_into_cache[d38c133e-a049-4599-98ff-a42147cfe624] received

As soon as I remove the above-mentioned keywords, the charts start working again. Has anyone faced a similar situation and could provide guidance on how to continue using these essential keywords in our context?

Screenshots/recordings

Image

Image

Image

Superset version

4.1.1

Python version

3.11

Node version

18 or greater

Browser

Chrome

Additional context

Works perfectly on 4.0.2 but crash on 4.1.1.rc2 (works on rc1 )
Seems to be a cache retreiving issue.
Works okay for all basic query but crash when using url_params jinja function.
Anyone have face this issues so far ? Maybe something related to #29840 as it touch cache key

Checklist

  • I have searched Superset docs and Slack and didn't find a solution to my problem.
  • I have searched the GitHub issue tracker and didn't find a similar bug report.
  • I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.
Copy link

dosubot bot commented Jan 23, 2025

I found a similar unsolved discussion that might be relevant to your issue: 4.1.1 migration troubleshooting with url_param() current_username() keywords in sql dataset, which was last updated on January 22, 2025 [1].

Regarding your issue, the GLOBAL_ASYNC_QUERIES flag in Superset 4.1.1 affects cache key retrieval by enabling asynchronous query execution. When this flag is enabled, Superset first attempts to retrieve the chart query results from the cache. If the results are not found, a background job is initiated to run the chart query [2].

The current_username() and url_param() functions interact with the caching system by including their values in the cache key calculation. This behavior can be controlled by setting add_to_cache_keys=False in your Jinja code, which might help in resolving the cache retrieval issues you're facing [3][4].

It might be beneficial to review the changes made in the GitHub pull request #29840, as it addresses issues related to cache key retrieval, particularly when using Jinja functions like current_username(), cache_key_wrapper(), and url_param(). This could be relevant to the problems you're experiencing with cache retrieval when the GLOBAL_ASYNC_QUERIES flag is set to True.

To continue talking to Dosu, mention @dosu.


Help Dosu learn! Give it feedback: Great Response | Irrelevant Answer | Incorrect Sources | Too Verbose | Hallucination | Other

@dosubot dosubot bot added global:async-query Related to Async Queries feature global:jinja Related to Jinja templating labels Jan 23, 2025
@ERGO1995
Copy link
Author

ERGO1995 commented Jan 23, 2025

@dosu Doesn't work

@ERGO1995
Copy link
Author

We tried to deploy 4.1.1 and revert #29840, then it works! But not a solution for us

Copy link

dosubot bot commented Jan 23, 2025

To resolve cache retrieval issues in Superset 4.1.1 when using current_username() and url_param() in SQL datasets with the GLOBAL_ASYNC_QUERIES flag, ensure these functions are correctly included in the cache key calculation. The ExtraCache class in superset/jinja_context.py is responsible for this.

  1. Ensure add_to_cache_keys is True: When using current_username() or url_param(), set add_to_cache_keys to True to include their results in the cache key.

    def current_username(self, add_to_cache_keys: bool = True) -> Optional[str]:
        if username := get_username():
            if add_to_cache_keys:
                self.cache_key_wrapper(username)
            return username
        return None
    
    def url_param(
        self,
        param: str,
        default: Optional[str] = None,
        add_to_cache_keys: bool = True,
        escape_result: bool = True,
    ) -> Optional[str]:
        # ... existing logic ...
        if add_to_cache_keys:
            self.cache_key_wrapper(result)
        return result
  2. Use cache_key_wrapper: This method appends values to the extra_cache_keys list, crucial for cache key calculation.

By following these steps, you can help ensure that cache keys are correctly calculated, which should resolve the cache retrieval issues [1].

To continue talking to Dosu, mention @dosu.


Help Dosu learn! Give it feedback: Great Response | Irrelevant Answer | Incorrect Sources | Too Verbose | Hallucination | Other

@ERGO1995
Copy link
Author

I found this update in superset/connectors/sqla/models.py in this commit 9023b48:
now: return list(set(extra_cache_keys))
before: return extra_cache_keys

If i only revert this line, it works again for us in 4.1.1. Can someone explain why did this change?

@rusackas
Copy link
Member

Tagging @villebro as he uses the GAQ flag, and I don't know anyone else using it in production (yet!).

Git blame shows @betodealmeida changed that line return list(set(extra_cache_keys)), so they may be able to speak to the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
global:async-query Related to Async Queries feature global:jinja Related to Jinja templating
Projects
None yet
Development

No branches or pull requests

4 participants