From 3f08e6e99f60802c777671d3d0422e6b18521e5c Mon Sep 17 00:00:00 2001 From: Taylor Steinberg Date: Tue, 23 Jul 2024 16:31:19 -0400 Subject: [PATCH] refactor: remove refresh and replace refresh with render (#238) --- .../tests/posit/connect/test_content.py | 14 +- src/posit/connect/content.py | 145 +++++++----------- tests/posit/connect/test_content.py | 38 ++++- 3 files changed, 91 insertions(+), 106 deletions(-) diff --git a/integration/tests/posit/connect/test_content.py b/integration/tests/posit/connect/test_content.py index 17df9e96..633d79b6 100644 --- a/integration/tests/posit/connect/test_content.py +++ b/integration/tests/posit/connect/test_content.py @@ -42,7 +42,7 @@ def test_content_item_owner_from_include(self): assert owner.guid == self.client.me.guid @pytest.mark.skipif( - CONNECT_VERSION <= version.parse("2023.01.1"), + CONNECT_VERSION <= version.parse("2024.04.1"), reason="Python 3.12 not available", ) def test_restart(self): @@ -57,7 +57,7 @@ def test_restart(self): # deploy bundle task = bundle.deploy() task.wait_for() - # restart content + # restart content.restart() # delete content content.delete() @@ -66,7 +66,7 @@ def test_restart(self): CONNECT_VERSION <= version.parse("2023.01.1"), reason="Quarto not available", ) - def test_refresh(self): + def test_render(self): # create content content = self.client.content.create(name="example-quarto-minimal") # create bundle @@ -78,10 +78,8 @@ def test_refresh(self): # deploy bundle task = bundle.deploy() task.wait_for() - # refresh content - task = content.refresh() - if task: - task.wait_for() - + # render + task = content.render() + task.wait_for() # delete content content.delete() diff --git a/src/posit/connect/content.py b/src/posit/connect/content.py index 53f7b263..64457660 100644 --- a/src/posit/connect/content.py +++ b/src/posit/connect/content.py @@ -3,7 +3,6 @@ from __future__ import annotations import secrets -import warnings from posixpath import dirname from typing import List, Optional, overload @@ -171,43 +170,80 @@ def deploy(self) -> tasks.Task: ts = tasks.Tasks(self.config, self.session) return ts.get(result["task_id"]) - def refresh(self) -> Task | None: - """Trigger a content refresh. + def render(self) -> Task: + """Render the content. - Submit a refresh request to the server for the content. After submission, the server executes an asynchronous process to refresh the content. This is useful when content is dependent on external information, such as a dataset which has been updated. + Submit a render request to the server for the content. After submission, the server executes an asynchronous process to render the content. This is useful when content is dependent on external information, such as a dataset. See Also -------- restart - Notes - ----- - This method is identical to `restart` and exists to provide contextual clarity. Both methods produce identical results. When working with documents, natural language prefers "refresh this content" instead of "restart this content" since documents do not require a system process. When writing software that operates on multiple types of content (e.g., applications, documents, scripts, etc.), you may use either 'refresh' or 'restart' to achive the same result. - Examples -------- - >>> refresh() + >>> render() """ - return self._re_whatever() + self.update() + + if self.app_mode in { + "rmd-static", + "jupyter-static", + "quarto-static", + }: + variants = self._variants.find() + variants = [variant for variant in variants if variant.is_default] + if len(variants) != 1: + raise RuntimeError( + f"Found {len(variants)} default variants. Expected 1. Without a single default variant, the content cannot be refreshed. This is indicative of a corrupted state." + ) + variant = variants[0] + return variant.render() + else: + raise ValueError( + f"Restart not supported for this application mode. Found {self.app_mode}" + ) - def restart(self) -> Task | None: - """Initiate a content restart. + def restart(self) -> None: + """Mark for restart. Sends a restart request to the server for the content. Once submitted, the server performs an asynchronous process to restart the content. This is particularly useful when the content relies on external information loaded into application memory, such as datasets. Additionally, restarting can help clear memory leaks or reduce excessive memory usage that might build up over time. See Also -------- - refresh - - Notes - ----- - This method is identical to `refresh` and exists to provide contextual clarity. Both methods produce identical results. When working with applications, natural language prefers "restart this content" instead of "refresh this content" since applications require a system process. When writing software that operates on multiple types of content (e.g., applications, documents, scripts, etc.), you may use either 'restart' or 'refresh' to achieve the same result. + render Examples -------- >>> restart() """ - return self._re_whatever() + self.update() + + if self.app_mode in { + "api", + "jupyter-voila", + "python-api", + "python-bokeh", + "python-dash", + "python-fastapi", + "python-shiny", + "python-streamlit", + "quarto-shiny", + "rmd-shiny", + "shiny", + "tensorflow-saved-model", + }: + random_hash = secrets.token_hex(32) + key = f"_CONNECT_RESTART_TMP_{random_hash}" + self.environment_variables.create(key, random_hash) + self.environment_variables.delete(key) + # GET via the base Connect URL to force create a new worker thread. + url = urls.append(dirname(self.config.url), f"content/{self.guid}") + self.session.get(url) + return None + else: + raise ValueError( + f"Restart not supported for this application mode. Found {self.app_mode}" + ) @overload def update( @@ -282,79 +318,6 @@ def update(self, *args, **kwargs) -> None: response = self.session.patch(url, json=body) super().update(**response.json()) - def _re_whatever(self) -> Task | None: - """Submit a re-whatever request (i.e., restart, refresh, etc). - - A re-whatever is a catch-all term for restarting, refreshing, or re-whatever-ing the content requires to bounce it to a new state. - - refresh: - For content that require variants. Find the default variant and render it again. - - restart: - For content that require server threads. Toggle an unique environment variable and open the content, which activates a new server thread. - - Returns - ------- - Task | None: - A task for the content render when available, otherwise None - - Raises - ------ - RuntimeError - Found an incorrect number of default variants. - - Examples - -------- - >>> _re_whatever() - """ - # Update the item to its current state. - # The 'app_mode' is not set until a bundle is created and deployed. - # During the deployment process, the 'app_mode' is read from manifest.json and written to the database. - # Until this occurs the 'app_mode' will be 'unknown'. - self.update() - - if self.app_mode in { - "rmd-static", - "jupyter-static", - "quarto-static", - }: - variants = self._variants.find() - variants = [variant for variant in variants if variant.is_default] - if len(variants) != 1: - raise RuntimeError( - f"Found {len(variants)} default variants. Expected 1. Without a single default variant, the content cannot be refreshed. This is indicative of a corrupted state." - ) - variant = variants[0] - return variant.render() - - if self.app_mode in { - "api", - "jupyter-voila", - "python-api", - "python-bokeh", - "python-dash", - "python-fastapi", - "python-shiny", - "python-streamlit", - "quarto-shiny", - "rmd-shiny", - "shiny", - "tensorflow-saved-model", - }: - random_hash = secrets.token_hex(32) - key = f"_CONNECT_RESTART_TMP_{random_hash}" - self.environment_variables.create(key, random_hash) - self.environment_variables.delete(key) - # GET via the base Connect URL to force create a new worker thread. - url = urls.append(dirname(self.config.url), f"content/{self.guid}") - self.session.get(url) - return None - - warnings.warn( - f"Content '{self.guid}' with application mode '{self.app_mode}' does not require restarts" - ) - return None - # Relationships @property diff --git a/tests/posit/connect/test_content.py b/tests/posit/connect/test_content.py index 6b58c89d..ab378574 100644 --- a/tests/posit/connect/test_content.py +++ b/tests/posit/connect/test_content.py @@ -527,7 +527,7 @@ def test(self): assert count == 3 -class TestRefresh: +class TestRender: @responses.activate def test(self): # data @@ -559,7 +559,7 @@ def test(self): content = c.content.get(guid) # invoke - task = content.refresh() + task = content.render() # assert assert task is not None @@ -568,6 +568,32 @@ def test(self): assert get_variants.call_count == 1 assert post_render.call_count == 1 + @responses.activate + def test_app_mode_is_other(self): + # data + guid = "f2f37341-e21d-3d80-c698-a935ad614066" + fixture_content = load_mock(f"v1/content/{guid}.json") + fixture_content.update(app_mode="other") + + # behavior + responses.get( + f"https://connect.example.com/__api__/v1/content/{guid}", + json=fixture_content, + ) + + responses.patch( + f"https://connect.example.com/__api__/v1/content/{guid}", + json=fixture_content, + ) + + # setup + c = Client("https://connect.example.com", "12345") + content = c.content.get(guid) + + # invoke + with pytest.raises(ValueError): + content.render() + @responses.activate def test_missing_default(self): responses.get( @@ -592,7 +618,7 @@ def test_missing_default(self): c = Client("https://connect.example.com", "12345") content = c.content.get("f2f37341-e21d-3d80-c698-a935ad614066") with pytest.raises(RuntimeError): - content.refresh() + content.render() class TestRestart: @@ -659,11 +685,9 @@ def test_app_mode_is_other(self): content = c.content.get(guid) # invoke - with warnings.catch_warnings(): - warnings.simplefilter("ignore", UserWarning) - task = content.restart() + with pytest.raises(ValueError): + content.restart() # assert - assert task is None assert mock_get_content.call_count == 1 assert mock_patch_content.call_count == 1