Make it more unlikely for content builds to get stuck in a running state

Author: dbkegley

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Added the name of the environment variables to the help output for those options that
   use environment variables as a default value.
 - Added support for deploying Shiny Express applications.
+- Added `--retry` and `--running` flags to the `rsconnect content build run` command.
 
 ### Changed
 - Improved the error and warning outputs when options conflict by providing the source
@@ -19,6 +20,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Updated verbose mode to output the source of all options being used when processing the
   CLI command.
 
+### Fixed
+- Interrupting a long-running `rsconnect content build run` command with `^C`
+  will now update the local state file before attempting graceful cleanup. This
+  should help prevent users from getting stuck a "build already running" state.
+  See [#467](https://github.com/rstudio/rsconnect-python/issues/467) for details.
+
 ## [1.21.0] - 2023-10-26
 
 ### Fixed

Makefile

@@ -55,7 +55,7 @@ mock-test-%: clean-stores
 	trap "$(MAKE) -C mock_connect down" EXIT; \
 	CONNECT_CONTENT_BUILD_DIR="rsconnect-build-test" \
 	CONNECT_SERVER="http://$(HOSTNAME):3939" \
-	CONNECT_API_KEY="0123456789abcdef0123456789abcdef" \
+	CONNECT_API_KEY="21232f297a57a5a743894a0e4a801fc3" \
 	$(MAKE) test-$*
 	@$(MAKE) -C mock_connect down
 

mock_connect/data.json

@@ -14,8 +14,8 @@
   ],
   "users": [
     {
-      "api_key": "0123456789abcdef0123456789abcdef",
-      "guid": "29a74070-2c13-4ef9-a898-cfc6bcf0f275",
+      "api_key": "21232f297a57a5a743894a0e4a801fc3",
+      "guid": "edf26318-0027-4d9d-bbbb-54703ebb1855",
       "username": "admin",
       "first_name": "Super",
       "last_name": "User",

rsconnect/actions_content.py

@@ -93,7 +93,17 @@ def build_history(connect_server, guid):
     return _content_build_store.get_build_history(guid)
 
 
-def build_start(connect_server, parallelism, aborted=False, error=False, all=False, poll_wait=2, debug=False):
+def build_start(
+    connect_server,
+    parallelism,
+    aborted=False,
+    error=False,
+    running=False,
+    retry=False,
+    all=False,
+    poll_wait=2,
+    debug=False,
+):
     init_content_build_store(connect_server)
     if _content_build_store.get_build_running():
         raise RSConnectException("There is already a build running on this server: %s" % connect_server.url)
@@ -105,6 +115,12 @@ def build_start(connect_server, parallelism, aborted=False, error=False, all=Fal
         all_content = list(map(lambda x: ContentGuidWithBundle(x["guid"], x["bundle_id"]), all_content))
         build_add_content(connect_server, all_content)
     else:
+        # --retry is shorthand for --aborted --error --running
+        if retry:
+            aborted = True
+            error = True
+            running = True
+
         aborted_content = []
         if aborted:
             logger.info("Adding ABORTED content to build...")
@@ -115,8 +131,14 @@ def build_start(connect_server, parallelism, aborted=False, error=False, all=Fal
             logger.info("Adding ERROR content to build...")
             error_content = _content_build_store.get_content_items(status=BuildStatus.ERROR)
             error_content = list(map(lambda x: ContentGuidWithBundle(x["guid"], x["bundle_id"]), error_content))
-        if len(aborted_content + error_content) > 0:
-            build_add_content(connect_server, aborted_content + error_content)
+        running_content = []
+        if running:
+            logger.info("Adding RUNNING content to build...")
+            running_content = _content_build_store.get_content_items(status=BuildStatus.RUNNING)
+            running_content = list(map(lambda x: ContentGuidWithBundle(x["guid"], x["bundle_id"]), running_content))
+
+        if len(aborted_content + error_content + running_content) > 0:
+            build_add_content(connect_server, aborted_content + error_content + running_content)
 
     content_items = _content_build_store.get_content_items(status=BuildStatus.NEEDS_BUILD)
     if len(content_items) == 0:
@@ -172,13 +194,28 @@ def build_start(connect_server, parallelism, aborted=False, error=False, all=Fal
             exit(1)
     except KeyboardInterrupt:
         ContentBuildStore._BUILD_ABORTED = True
+        logger.info("Content build interrupted...")
+        logger.info(
+            "Content that was in the RUNNING state may still be building on the "
+            + "Connect server. Server builds will not be interrupted."
+        )
+        logger.info(
+            "To find content items that _may_ still be running on the server, "
+            + "use: rsconnect content build ls --status RUNNING"
+        )
+        logger.info(
+            "To retry the content build, including items that were interrupted "
+            + "or failed, use: rsconnect content build run --retry"
+        )
     finally:
+        # make sure that we always mark the build as complete but note
+        # there's no guarantee that the content_executor or build_monitor
+        # were allowed to shut down gracefully, they may have been interrupted.
+        _content_build_store.set_build_running(False)
         if content_executor:
             content_executor.shutdown(wait=False)
         if build_monitor:
             build_monitor.shutdown()
-        # make sure that we always mark the build as complete once we finish our cleanup
-        _content_build_store.set_build_running(False)
 
 
 def _monitor_build(connect_server, content_items):

rsconnect/main.py

@@ -2386,6 +2386,13 @@ def get_build_logs(
 )
 @click.option("--aborted", is_flag=True, help="Build content that is in the ABORTED state.")
 @click.option("--error", is_flag=True, help="Build content that is in the ERROR state.")
+@click.option("--running", is_flag=True, help="Build content that is in the RUNNING state.")
+@click.option(
+    "--retry",
+    is_flag=True,
+    help="Build all content that is in the NEEDS_BUILD, ABORTED, ERROR, or RUNNING state. "
+    + "Shorthand for `--aborted --error --running`.",
+)
 @click.option("--all", is_flag=True, help="Build all content, even if it is already marked as COMPLETE.")
 @click.option(
     "--poll-wait",
@@ -2416,6 +2423,8 @@ def start_content_build(
     parallelism: int,
     aborted: bool,
     error: bool,
+    running: bool,
+    retry: bool,
     all: bool,
     poll_wait: float,
     format: str,
@@ -2427,7 +2436,7 @@ def start_content_build(
     logger.set_log_output_format(format)
     with cli_feedback("", stderr=True):
         ce = RSConnectExecutor(ctx, name, server, api_key, insecure, cacert, logger=None).validate_server()
-        build_start(ce.remote_server, parallelism, aborted, error, all, poll_wait, debug)
+        build_start(ce.remote_server, parallelism, aborted, error, running, retry, all, poll_wait, debug)
 
 
 @cli.group(no_args_is_help=True, help="Interact with Posit Connect's system API.")

tests/test_main_content.py

@@ -7,8 +7,9 @@
 
 from rsconnect.main import cli
 from rsconnect import VERSION
+from rsconnect.api import RSConnectServer
 from rsconnect.models import BuildStatus
-from rsconnect.metadata import _normalize_server_url
+from rsconnect.metadata import ContentBuildStore, _normalize_server_url
 
 from .utils import apply_common_args, require_api_key, require_connect
 
@@ -116,6 +117,51 @@ def test_build(self):
         self.assertTrue(len(listing) == 1)
         self.assertEqual(listing[0]["rsconnect_build_status"], BuildStatus.COMPLETE)
 
+    def test_build_retry(self):
+        connect_server = require_connect()
+        api_key = require_api_key()
+        runner = CliRunner()
+
+        # add a content item
+        args = ["content", "build", "add", "-g", _content_guids[0]]
+        apply_common_args(args, server=connect_server, key=api_key)
+        result = runner.invoke(cli, args)
+        self.assertEqual(result.exit_code, 0, result.output)
+        self.assertTrue(
+            os.path.exists("%s/%s.json" % (_test_build_dir, _normalize_server_url(os.environ.get("CONNECT_SERVER"))))
+        )
+
+        # list the "tracked" content
+        args = ["content", "build", "ls", "-g", _content_guids[0]]
+        apply_common_args(args, server=connect_server, key=api_key)
+        result = runner.invoke(cli, args)
+        self.assertEqual(result.exit_code, 0, result.output)
+        listing = json.loads(result.output)
+        self.assertTrue(len(listing) == 1)
+        self.assertEqual(listing[0]["guid"], _content_guids[0])
+        self.assertEqual(listing[0]["bundle_id"], "176")
+        self.assertEqual(listing[0]["rsconnect_build_status"], BuildStatus.NEEDS_BUILD)
+
+        # set the content build status to RUNNING so it looks like it was interrupted
+        # and the cleanup did not have time to finish, otherwise it would be marked as ABORTED
+        store = ContentBuildStore(RSConnectServer(connect_server, api_key))
+        store.set_content_item_build_status(_content_guids[0], BuildStatus.RUNNING)
+
+        # run the build
+        args = ["content", "build", "run", "--retry"]
+        apply_common_args(args, server=connect_server, key=api_key)
+        result = runner.invoke(cli, args)
+        self.assertEqual(result.exit_code, 0, result.output)
+
+        # check that the build succeeded
+        args = ["content", "build", "ls", "-g", _content_guids[0]]
+        apply_common_args(args, server=connect_server, key=api_key)
+        result = runner.invoke(cli, args)
+        self.assertEqual(result.exit_code, 0, result.output)
+        listing = json.loads(result.output)
+        self.assertTrue(len(listing) == 1)
+        self.assertEqual(listing[0]["rsconnect_build_status"], BuildStatus.COMPLETE)
+
     def test_build_rm(self):
         connect_server = require_connect()
         api_key = require_api_key()