Remove experimental cached widget replay logic (streamlit#9305)

## Describe your changes Removes the deprecated experimental cached widget replay feature logic (`experimental_allow_widgets`). We will still keep the parameter around for a couple of releases. ## Testing Plan - Just removes logic -> no new tests needed. --- **Contribution License Agreement** By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.
souvikmaji · Aug 20, 2024 · c837c2e · c837c2e
1 parent ef630f4
commit c837c2e
Show file tree

Hide file tree

Showing 21 changed files with 142 additions and 663 deletions.
diff --git a/e2e_playwright/st_cache_data.py b/e2e_playwright/st_cache_data.py
@@ -17,22 +17,6 @@
 
 import streamlit as st
 
-st.button("click to rerun")
-
-side_effects = []
-
-
-@st.cache_data(experimental_allow_widgets=True)
-def foo():
-    side_effects.append("function ran")
-    r = st.radio("radio", ["foo", "bar", "baz", "qux"], index=1)
-    return r
-
-
-foo()
-
-st.text(side_effects)
-
 
 @st.cache_data
 def with_cached_widget_warning():
@@ -44,21 +28,22 @@ def with_cached_widget_warning():
     with_cached_widget_warning()
 
 
-@st.cache_data(experimental_allow_widgets=True)
+@st.cache_data
 def inner_cache_function():
     st.radio("radio 2", ["foo", "bar", "baz", "qux"], index=1)
 
 
-@st.cache_data(experimental_allow_widgets=False)
+@st.cache_data
 def nested_cached_function():
     inner_cache_function()
     st.selectbox("selectbox 2", ["foo", "bar", "baz", "qux"], index=1)
 
 
 if st.button("Run nested cached function with widget warning"):
-    # When running nested_cached_function(), we get two warnings, one from nested_cached_function()
-    # and one from inner_cache_function. inner_cache_function() on its own would allow the
-    # widget usage, but since it is nested in the other function that does not allow it, we don't allow it.
+    # When running nested_cached_function(), we get two warnings, one from
+    # nested_cached_function() and one from inner_cache_function. inner_cache_function()
+    # on its own would allow the widget usage, but since it is nested in the other
+    # function that does not allow it, we don't allow it.
     # The outer experimental_allow_widgets=False will always take priority.
     # Otherwise, we would need to recompute the outer cached function whenever
     # the widget in the inner function is used. Which we don't want to do when

diff --git a/e2e_playwright/st_cache_data_test.py b/e2e_playwright/st_cache_data_test.py
@@ -18,32 +18,11 @@
 from playwright.sync_api import Page, expect
 
 from e2e_playwright.conftest import rerun_app, wait_for_app_run
-from e2e_playwright.shared.app_utils import click_checkbox, get_image
-
-
-def test_runs_cached_function_with_new_widget_values(app: Page):
-    expect(app.get_by_test_id("stRadio")).to_have_count(1)
-    expect(app.get_by_test_id("stText")).to_have_text("['function ran']")
-    app.get_by_text("click to rerun").click()
-
-    wait_for_app_run(app)
-    expect(app.get_by_test_id("stRadio")).to_have_count(1)
-    expect(app.get_by_test_id("stText")).to_have_text("[]")
-
-    app.get_by_test_id("stRadio").nth(0).locator(
-        'label[data-baseweb="radio"]'
-    ).last.click()
-    expect(app.get_by_test_id("stRadio")).to_have_count(1)
-    expect(app.get_by_test_id("stText")).to_have_text("['function ran']")
-    app.get_by_text("click to rerun").click()
-
-    wait_for_app_run(app)
-    expect(app.get_by_test_id("stRadio")).to_have_count(1)
-    expect(app.get_by_test_id("stText")).to_have_text("[]")
+from e2e_playwright.shared.app_utils import click_button, click_checkbox, get_image
 
 
 def test_that_caching_shows_cached_widget_warning(app: Page):
-    app.get_by_text("Run cached function with widget warning").click()
+    click_button(app, "Run cached function with widget warning")
     wait_for_app_run(app)
     expect(app.get_by_test_id("stException")).to_have_count(1)
 
@@ -52,8 +31,7 @@ def test_that_caching_shows_cached_widget_warning(app: Page):
 
 
 def test_that_nested_cached_function_shows_cached_widget_warning(app: Page):
-    app.get_by_text("Run nested cached function with widget warning").click()
-    wait_for_app_run(app)
+    click_button(app, "Run nested cached function with widget warning")
     expect(app.get_by_test_id("stException")).to_have_count(2)
 
     expect(app.get_by_test_id("stException").nth(0)).to_contain_text(
@@ -65,15 +43,13 @@ def test_that_nested_cached_function_shows_cached_widget_warning(app: Page):
 
 
 def test_that_replay_element_works_as_expected(app: Page):
-    app.get_by_text("Cached function with element replay").click()
-    wait_for_app_run(app)
+    click_button(app, "Cached function with element replay")
     expect(app.get_by_test_id("stException")).to_have_count(0)
     expect(app.get_by_text("Cache executions: 1")).to_be_visible()
     expect(app.get_by_text("Cache return 1")).to_be_visible()
 
     # Execute again, the values should be the same:
-    app.get_by_text("Cached function with element replay").click()
-    wait_for_app_run(app)
+    click_button(app, "Cached function with element replay")
     expect(app.get_by_test_id("stException")).to_have_count(0)
     expect(app.get_by_text("Cache executions: 1")).to_be_visible()
     expect(app.get_by_text("Cache return 1")).to_be_visible()

diff --git a/e2e_playwright/st_cache_resource.py b/e2e_playwright/st_cache_resource.py
@@ -14,22 +14,6 @@
 
 import streamlit as st
 
-st.button("click to rerun")
-
-side_effects = []
-
-
-@st.cache_resource(experimental_allow_widgets=True)
-def foo():
-    side_effects.append("function ran")
-    r = st.radio("radio", ["foo", "bar", "baz", "qux"], index=1)
-    return r
-
-
-foo()
-
-st.text(side_effects)
-
 
 @st.cache_resource
 def with_cached_widget_warning():
@@ -41,21 +25,22 @@ def with_cached_widget_warning():
     with_cached_widget_warning()
 
 
-@st.cache_resource(experimental_allow_widgets=True)
+@st.cache_resource
 def inner_cache_function():
     st.radio("radio 2", ["foo", "bar", "baz", "qux"], index=1)
 
 
-@st.cache_resource(experimental_allow_widgets=False)
+@st.cache_resource
 def nested_cached_function():
     inner_cache_function()
     st.selectbox("selectbox 2", ["foo", "bar", "baz", "qux"], index=1)
 
 
 if st.button("Run nested cached function with widget warning"):
-    # When running nested_cached_function(), we get two warnings, one from nested_cached_function()
-    # and one from inner_cache_function. inner_cache_function() on its own would allow the
-    # widget usage, but since it is nested in the other function that does not allow it, we don't allow it.
+    # When running nested_cached_function(), we get two warnings, one from
+    # nested_cached_function() and one from inner_cache_function. inner_cache_function()
+    # on its own would allow the widget usage, but since it is nested in the other
+    # function that does not allow it, we don't allow it.
     # The outer experimental_allow_widgets=False will always take priority.
     # Otherwise, we would need to recompute the outer cached function whenever
     # the widget in the inner function is used. Which we don't want to do when

diff --git a/e2e_playwright/st_cache_resource_test.py b/e2e_playwright/st_cache_resource_test.py
@@ -15,42 +15,19 @@
 
 from playwright.sync_api import Page, expect
 
-from e2e_playwright.conftest import wait_for_app_run
-
-
-def test_runs_cached_function_with_new_widget_values(app: Page):
-    expect(app.get_by_test_id("stRadio")).to_have_count(1)
-    expect(app.get_by_test_id("stText")).to_have_text("['function ran']")
-    app.get_by_text("click to rerun").click()
-
-    wait_for_app_run(app)
-    expect(app.get_by_test_id("stRadio")).to_have_count(1)
-    expect(app.get_by_test_id("stText")).to_have_text("[]")
-
-    app.get_by_test_id("stRadio").nth(0).locator(
-        'label[data-baseweb="radio"]'
-    ).last.click()
-    expect(app.get_by_test_id("stRadio")).to_have_count(1)
-    expect(app.get_by_test_id("stText")).to_have_text("['function ran']")
-    app.get_by_text("click to rerun").click()
-
-    wait_for_app_run(app)
-    expect(app.get_by_test_id("stRadio")).to_have_count(1)
-    expect(app.get_by_test_id("stText")).to_have_text("[]")
+from e2e_playwright.shared.app_utils import click_button
 
 
 def test_that_caching_shows_cached_widget_warning(app: Page):
-    app.get_by_text("Run cached function with widget warning").click()
-    wait_for_app_run(app)
+    click_button(app, "Run cached function with widget warning")
     expect(app.get_by_test_id("stException")).to_have_count(1)
 
     exception_element = app.get_by_test_id("stException").nth(0)
     expect(exception_element).to_contain_text("CachedWidgetWarning: Your script uses")
 
 
 def test_that_nested_cached_function_shows_cached_widget_warning(app: Page):
-    app.get_by_text("Run nested cached function with widget warning").click()
-    wait_for_app_run(app)
+    click_button(app, "Run nested cached function with widget warning")
     expect(app.get_by_test_id("stException")).to_have_count(2)
 
     expect(app.get_by_test_id("stException").nth(0)).to_contain_text(
@@ -62,15 +39,13 @@ def test_that_nested_cached_function_shows_cached_widget_warning(app: Page):
 
 
 def test_that_replay_element_works_as_expected(app: Page):
-    app.get_by_text("Cached function with element replay").click()
-    wait_for_app_run(app)
+    click_button(app, "Cached function with element replay")
     expect(app.get_by_test_id("stException")).to_have_count(0)
     expect(app.get_by_text("Cache executions: 1")).to_be_visible()
     expect(app.get_by_text("Cache return 1")).to_be_visible()
 
     # Execute again, the values should be the same:
-    app.get_by_text("Cached function with element replay").click()
-    wait_for_app_run(app)
+    click_button(app, "Cached function with element replay")
     expect(app.get_by_test_id("stException")).to_have_count(0)
     expect(app.get_by_text("Cache executions: 1")).to_be_visible()
     expect(app.get_by_text("Cache return 1")).to_be_visible()
diff --git a/lib/streamlit/elements/lib/policies.py b/lib/streamlit/elements/lib/policies.py
@@ -19,7 +19,10 @@
 from streamlit import config, errors, logger, runtime
 from streamlit.elements.form_utils import is_in_form
 from streamlit.errors import StreamlitAPIException, StreamlitAPIWarning
-from streamlit.runtime.scriptrunner_utils.script_run_context import get_script_run_ctx
+from streamlit.runtime.scriptrunner_utils.script_run_context import (
+    get_script_run_ctx,
+    in_cached_function,
+)
 from streamlit.runtime.state import WidgetCallback, get_session_state
 
 if TYPE_CHECKING:
@@ -114,14 +117,12 @@ def check_cache_replay_rules() -> None:
     If there are other similar checks in the future, we could extend this
     function to check for those as well. And rename it to check_widget_usage_rules.
     """
-    if runtime.exists():
-        ctx = get_script_run_ctx()
-        if ctx and ctx.disallow_cached_widget_usage:
-            from streamlit import exception
-
-            # We use an exception here to show a proper stack trace
-            # that indicates to the user where the issue is.
-            exception(CachedWidgetWarning())
+    if in_cached_function.get():
+        from streamlit import exception
+
+        # We use an exception here to show a proper stack trace
+        # that indicates to the user where the issue is.
+        exception(CachedWidgetWarning())
 
 
 _fragment_writes_widget_to_outside_error = (

diff --git a/lib/streamlit/runtime/caching/__init__.py b/lib/streamlit/runtime/caching/__init__.py
@@ -14,7 +14,7 @@
 
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING
 
 from streamlit.runtime.caching.cache_data_api import (
     CACHE_DATA_MESSAGE_REPLAY_CTX,
@@ -33,7 +33,6 @@
     from google.protobuf.message import Message
 
     from streamlit.proto.Block_pb2 import Block
-    from streamlit.runtime.state.common import WidgetMetadata
 
 
 def save_element_message(
@@ -73,14 +72,6 @@ def save_block_message(
     )
 
 
-def save_widget_metadata(metadata: WidgetMetadata[Any]) -> None:
-    """Save a widget's metadata to a thread-local callstack, so the widget
-    can be registered again when that widget is replayed.
-    """
-    CACHE_DATA_MESSAGE_REPLAY_CTX.save_widget_metadata(metadata)
-    CACHE_RESOURCE_MESSAGE_REPLAY_CTX.save_widget_metadata(metadata)
-
-
 def save_media_data(image_data: bytes | str, mimetype: str, image_id: str) -> None:
     CACHE_DATA_MESSAGE_REPLAY_CTX.save_image_data(image_data, mimetype, image_id)
     CACHE_RESOURCE_MESSAGE_REPLAY_CTX.save_image_data(image_data, mimetype, image_id)
@@ -99,7 +90,6 @@ def save_media_data(image_data: bytes | str, mimetype: str, image_id: str) -> No
     "CACHE_DOCS_URL",
     "save_element_message",
     "save_block_message",
-    "save_widget_metadata",
     "save_media_data",
     "get_data_cache_stats_provider",
     "get_resource_cache_stats_provider",