diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 6c0b4604..5f53b154 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -8,21 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased - Posit Connect Cloud is now a supported deployment target, alongside Posit - Connect and shinyapps.io, mirroring the R rsconnect package's support. - Register an account with `rsconnect add -n --connect-cloud - -A `, which verifies the account exists and that you can publish to - it. Authentication is an interactive browser login by default; for CI and - other non-interactive use, pass a service account credential with - `--client-id`/`--client-secret` (or the `CONNECT_CLOUD_CLIENT_ID` and - `CONNECT_CLOUD_CLIENT_SECRET` environment variables). Tokens are refreshed - automatically. Deploy with any `rsconnect deploy` subcommand by passing - `--connect-cloud` (or `-s connect.posit.cloud`) with `-A ` (or the - `CONNECT_CLOUD_ACCOUNT` environment variable; a `SHINYAPPS_ACCOUNT` variable - exported for shinyapps.io is ignored here), or `-n ` for a saved - credential — which publishes to the account it was saved with, or to another - account of the same login when `-A` is given as well. Supported content types: - Shiny (Python and R), Streamlit, Dash, Bokeh, Jupyter notebooks, Quarto, - R Markdown, and static content. + Connect and shinyapps.io. - `rsconnect add` now reports invalid option combinations and unreadable certificate files as plain error messages. Previously these surfaced as raw Python tracebacks. diff --git a/rsconnect/api.py b/rsconnect/api.py index 7aa3b3d1..aa0bcd40 100644 --- a/rsconnect/api.py +++ b/rsconnect/api.py @@ -177,7 +177,7 @@ def handle_bad_response(self, response: HTTPResponse | T, is_httpresponse: bool response.json_data["error"], ) raise RSConnectException(error, status=response.status) - if response.status < 200 or response.status > 299: + if response.status is None or response.status < 200 or response.status > 299: raise RSConnectException( "Received an unexpected response from %s (calling %s): %s %s" % ( @@ -2254,6 +2254,10 @@ def should_deploy_as_draft(self, draft: bool, no_verify: bool) -> bool: With ``--no-verify`` we activate immediately. """ if draft: + if not isinstance(self.client, RSConnectClient): + # Neither Connect Cloud nor shinyapps.io has a draft step at any version, + # so the version below would send the reader looking for one. + raise RSConnectException("Deploying as a draft is only supported by Posit Connect.") if not self.supports_verify_before_activate: # We can't honor --draft without the activate field: silently activating # would be the opposite of what the user asked for, so fail loudly. @@ -3196,11 +3200,10 @@ def update_content( ) -> ConnectCloudContent: """Update content, optionally minting a fresh revision to upload into. - `content_type` and `primary_file` are always sent alongside `app_mode`: - the API only recomputes `app_mode` when one of them is present in the - override set, and the stored content type would otherwise survive a - redeploy that changes what kind of content this is (--app-id pointing at - content of another type). + `revision_overrides` is a partial set: any field left out keeps the value + already stored on the content. All three go every time so a redeploy that + changes what kind of content this is (--app-id pointing at content of + another type) does not keep the old content type. `secrets` replaces the content's whole set, and `title` overwrites the stored one, so both are omitted from the request entirely when None: a diff --git a/rsconnect/certificates.py b/rsconnect/certificates.py index 95b98714..a57be574 100644 --- a/rsconnect/certificates.py +++ b/rsconnect/certificates.py @@ -23,13 +23,8 @@ def read_certificate_file(location: str) -> str | bytes: path = Path(location) - # Readability is judged before the suffix so a bad path reports as the real - # problem: the file type of a file that does not exist is beside the point. - # Both are operational errors: the CLI no longer checks existence at parse - # time (the certificate only applies once the target is known), so this is - # where a bad path surfaces. is_file() sits inside the handler because it - # raises OSError itself when the path's metadata cannot be read, e.g. - # through a permission-denied directory. + # is_file() is inside the try because it raises OSError itself when the path's + # metadata cannot be read, e.g. through a permission-denied directory. try: if not path.is_file(): raise RSConnectException("The certificate file %s could not be read: no such file." % location) diff --git a/rsconnect/metadata.py b/rsconnect/metadata.py index f08783dc..eacd1154 100644 --- a/rsconnect/metadata.py +++ b/rsconnect/metadata.py @@ -245,6 +245,21 @@ def save_to(self, path: str, data: bytes, open: Callable[..., BufferedWriter] = f.write(data) self._real_path = path + def _already_holds(self, path: str, data: bytes) -> bool: + """ + Whether this store was loaded from `path` and that file already holds `data`. + + Restricted to the path we read so a store loaded from its secondary + location still migrates to the primary one on the next save. + """ + if self._real_path != path or not exists(path): + return False + try: + with open(path, "rb") as f: + return f.read() == data + except OSError: + return False + # noinspection PyShadowingBuiltins def save(self, open: Callable[..., BufferedWriter] = open): """ @@ -252,19 +267,22 @@ def save(self, open: Callable[..., BufferedWriter] = open): The app directory is tried first. If that fails, then we write to the global config location. + + A save that would not change the file is skipped. """ data = json.dumps(self._data, indent=4).encode("utf-8") - try: - makedirs(self._primary_path) - self.save_to(self._primary_path, data, open) - except OSError: - if not self._secondary_path: - raise - makedirs(self._secondary_path) - self.save_to(self._secondary_path, data, open) - - if self._chmod and self._real_path is not None: - os.chmod(self._real_path, 0o600) + if not self._already_holds(self._primary_path, data): + try: + makedirs(self._primary_path) + self.save_to(self._primary_path, data, open) + except OSError: + if not self._secondary_path: + raise + makedirs(self._secondary_path) + self.save_to(self._secondary_path, data, open) + + if self._chmod and self._real_path is not None: + os.chmod(self._real_path, 0o600) class ServerDataDict(TypedDict): diff --git a/rsconnect/validation.py b/rsconnect/validation.py index 43791b9d..ce482462 100644 --- a/rsconnect/validation.py +++ b/rsconnect/validation.py @@ -60,6 +60,38 @@ def _get_present_options( return result +# Deploy options that configure Posit Connect features Connect Cloud does not +# have. +_CONNECT_ONLY_DEPLOY_OPTIONS: dict[str, str] = { + "image": "-I/--image", + "disable_env_management": "--disable-env-management", + "env_management_py": "--disable-env-management-py", + "env_management_r": "--disable-env-management-r", + "draft": "--draft", + "metadata": "--metadata", +} + + +def _typed_connect_only_deploy_options(ctx: Optional[click.Context]) -> list[str]: + """The Connect-only deploy options this command line passed. + + Judged by parameter source rather than by value: --disable-env-management-py + inverts to False when given, and the --disable-env-management shorthand fills + in the per-language parameters without being their source. + + Only options count. `environment add` takes a positional IMAGE argument, whose + parameter is also named `image` and is not this option. + """ + if ctx is None: + return [] + option_names = {param.name for param in ctx.command.params if isinstance(param, click.Option)} + return [ + label + for name, label in _CONNECT_ONLY_DEPLOY_OPTIONS.items() + if name in option_names and get_parameter_source_name_from_ctx(name, ctx) == "COMMANDLINE" + ] + + def validate_connect_cloud_incompatible_options( ctx: Optional[click.Context], api_key: Optional[str], @@ -69,12 +101,12 @@ def validate_connect_cloud_incompatible_options( ): """Reject options that have no meaning on Posit Connect Cloud. - validate_connection_options performs the same checks, but only when Connect - Cloud is selected by flag or URL; a saved nickname or default server is only - identified as Connect Cloud after store resolution, so the executor calls - this afterwards. Environment-sourced values are ignored for the same reason - as elsewhere: a CONNECT_API_KEY exported for another target is just the - CI environment, not a request to use it here. + validate_connection_options repeats the credential and SPCS checks, but only + when Connect Cloud is selected by flag or URL; a saved nickname or default + server is only identified as Connect Cloud after store resolution, so the + executor calls this afterwards. Environment-sourced values are ignored for + the same reason as elsewhere: a CONNECT_API_KEY exported for another target + is just the CI environment, not a request to use it here. """ present_connect_options = _get_present_options( {"-k/--api-key": api_key, "-i/--insecure": insecure, "-c/--cacert": cacert}, @@ -92,6 +124,12 @@ def validate_connect_cloud_incompatible_options( if present_spcs_options: raise RSConnectException( f"SPCS options ({', '.join(present_spcs_options)}) may not be passed \ +alongside Posit Connect Cloud. See command help for further details." + ) + connect_only_deploy_options = _typed_connect_only_deploy_options(ctx) + if connect_only_deploy_options: + raise RSConnectException( + f"Posit Connect options ({', '.join(connect_only_deploy_options)}) may not be passed \ alongside Posit Connect Cloud. See command help for further details." ) diff --git a/tests/test_connect_cloud.py b/tests/test_connect_cloud.py index 9a542160..3b9bd30e 100644 --- a/tests/test_connect_cloud.py +++ b/tests/test_connect_cloud.py @@ -30,12 +30,14 @@ from rsconnect.metadata import AppStore, ServerData, ServerStore from rsconnect.models import AppModes from rsconnect.oauth import InvalidClientError, InvalidGrantError +from rsconnect import validation from rsconnect.validation import validate_connection_options from .utils import failing_keyring ENV = ParameterSource.ENVIRONMENT TYPED = ParameterSource.COMMANDLINE +DEFAULT = ParameterSource.DEFAULT class TestConnectCloudEnvironments(unittest.TestCase): @@ -337,8 +339,13 @@ def _json_body(request): def _ctx(**sources: ParameterSource) -> click.Context: - """A click context recording where each named parameter's value came from.""" - ctx = click.Context(click.Command("deploy")) + """A click context recording where each named parameter's value came from. + + Each name is declared as an option, since validation distinguishes options + from same-named arguments. + """ + params: list[click.Parameter] = [click.Option(["--%s" % param.replace("_", "-")]) for param in sources] + ctx = click.Context(click.Command("deploy", params=params)) for param, source in sources.items(): ctx.set_parameter_source(param, source) # pyright: ignore[reportAttributeAccessIssue] return ctx @@ -665,9 +672,9 @@ def _update_content(self, **kwargs): return _json_body(httpretty.last_request()) @httpretty.activate(verbose=True, allow_net_connect=False) - def test_update_content_sends_primary_file_with_app_mode(self): - # The API only recomputes app_mode when content_type or primary_file is - # in the override set; sending app_mode alone would persist it verbatim. + def test_update_content_sends_the_whole_revision_override_set(self): + # Omitted overrides keep the content's stored value, so all three go every + # time or a redeploy of a different kind of content keeps the old ones. body = self._update_content() self.assertEqual( body["revision_overrides"], @@ -2985,6 +2992,73 @@ def test_remove_by_url_with_a_single_entry_removes_it(self): self.assertEqual(store.get_all_servers(), []) +class TestConnectOnlyDeployOptions(unittest.TestCase): + """Deploy options that configure Posit Connect features Connect Cloud does not + have. Connect Cloud ignores them, so they are rejected rather than accepted and + dropped.""" + + OPTIONS = { + "image": "-I/--image", + "disable_env_management": "--disable-env-management", + "env_management_py": "--disable-env-management-py", + "env_management_r": "--disable-env-management-r", + "draft": "--draft", + "metadata": "--metadata", + } + + def test_each_is_rejected_with_the_flag(self): + for param, label in self.OPTIONS.items(): + with self.subTest(param): + with self.assertRaises(RSConnectException) as context: + _setup_remote_server(ctx=_ctx(**{param: TYPED}), account_name="acme", use_connect_cloud=True) + message = str(context.exception) + self.assertIn(label, message) + self.assertIn("may not be passed alongside Posit Connect Cloud", message) + + def test_each_is_rejected_for_a_saved_cloud_nickname(self): + # A nickname is only known to name a Connect Cloud credential after the + # store lookup, which is why this is checked in the executor. + for param, label in self.OPTIONS.items(): + with self.subTest(param): + with self.assertRaises(RSConnectException) as context: + _setup_remote_server( + ctx=_ctx(**{param: TYPED}), resolve=_cloud_entry(connect_cloud_access_token="at"), name="cloud" + ) + self.assertIn(label, str(context.exception)) + + def test_defaulted_options_are_accepted(self): + # --disable-env-management-py inverts to False when given and the shorthand + # sets the same parameters without being their source, so what the user + # typed can only be read from the parameter source. + _cloud_server(ctx=_ctx(image=DEFAULT, env_management_py=DEFAULT), account_name="acme", use_connect_cloud=True) + + def test_a_connect_target_still_accepts_them(self): + store = ServerStore(base_dir=tempfile.mkdtemp()) + store.set("prod", "https://connect.example.com", api_key="key") + executor = _setup_remote_server(ctx=_ctx(image=TYPED), store=store, name="prod") + self.assertIsInstance(executor.remote_server, api.RSConnectServer) + + def test_draft_at_the_deploy_step_does_not_cite_a_connect_version(self): + # The CLI rejects --draft before this, but a programmatic caller has no + # click context to judge, and a Connect version says nothing to a target + # with no draft step. + executor = RSConnectExecutor.__new__(RSConnectExecutor) + executor.client = mock.Mock(spec=ConnectCloudClient) + with self.assertRaises(RSConnectException) as context: + executor.should_deploy_as_draft(draft=True, no_verify=False) + message = str(context.exception) + self.assertIn("only supported by Posit Connect", message) + self.assertNotIn("2025.06.0", message) + + def test_a_same_named_argument_is_not_one_of_these_options(self): + # `environment add` takes a positional IMAGE, so a Connect Cloud nickname + # must reach that command's own "requires a Posit Connect server" error + # rather than be told it passed -I/--image. + command = cli.commands["environment"].commands["add"] + with command.make_context("add", ["my-image:1.0", "-n", "cloud"]) as ctx: + self.assertEqual(validation._typed_connect_only_deploy_options(ctx), []) + + class TestConnectCloudFindsSavedCredentialsByUrl(unittest.TestCase): """`--connect-cloud` and `-s connect.posit.cloud` must find a saved server. diff --git a/tests/test_metadata.py b/tests/test_metadata.py index 849c1126..6b9ce52b 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -1,3 +1,4 @@ +import os import shutil import tempfile from os.path import exists, join @@ -253,6 +254,31 @@ def test_remove_default_clears_it(self): self.server_store.remove_by_name("foo") self.assertIsNone(self.server_store.get_default()) + def test_save_skips_rewrite_when_file_unchanged(self): + def fail_open(path_to_open, mode, *args, **kw): + self.fail("rewrote %s when nothing had changed" % path_to_open) + + self.server_store.save(fail_open) + + def test_save_rewrites_when_data_changed(self): + writes = [] + + def recording_open(path_to_open, mode, *args, **kw): + writes.append(path_to_open) + return open(path_to_open, mode, *args, **kw) + + del self.server_store._data["foo"] + self.server_store.save(recording_open) + + self.assertEqual(writes, [self.server_store_path]) + self.assertIsNone(ServerStore(base_dir=self.tempDir).get_by_name("foo")) + + def test_save_rewrites_when_file_removed(self): + os.remove(self.server_store_path) + self.server_store.save() + + self.assertEqual(len(ServerStore(base_dir=self.tempDir).get_all_servers()), 5) + class TestAppMetadata(TestCase): def setUp(self):