From 1f512b463bdc318d8ff1e7c4d81875518425ebcf Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Wed, 14 Feb 2024 12:08:33 -0600 Subject: [PATCH 01/23] Update type annotations --- rsconnect/api.py | 37 +++++++++++++++++-------------- rsconnect/bundle.py | 48 ++++++++++++++++++---------------------- rsconnect/environment.py | 11 ++++----- rsconnect/main.py | 12 +++++----- rsconnect/validation.py | 17 ++++++++------ 5 files changed, 63 insertions(+), 62 deletions(-) diff --git a/rsconnect/api.py b/rsconnect/api.py index 758ebf7f..f86bb5e0 100644 --- a/rsconnect/api.py +++ b/rsconnect/api.py @@ -1,11 +1,14 @@ """ Posit Connect API client and utility functions """ + +from __future__ import annotations + import binascii import os from os.path import abspath, dirname import time -from typing import IO, Callable +from typing import IO, Callable, Optional import base64 import datetime import hashlib @@ -390,17 +393,17 @@ def output_task_log(task_status, last_status, log_callback): class RSConnectExecutor: def __init__( self, - ctx: click.Context = None, - name: str = None, - url: str = None, - api_key: str = None, + ctx: Optional[click.Context] = None, + name: Optional[str] = None, + url: Optional[str] = None, + api_key: Optional[str] = None, insecure: bool = False, - cacert: str = None, - ca_data: str = None, + cacert: Optional[str] = None, + ca_data: Optional[str] = None, cookies=None, account=None, - token: str = None, - secret: str = None, + token: Optional[str] = None, + secret: Optional[str] = None, timeout: int = 30, logger=console_logger, **kwargs @@ -471,15 +474,15 @@ def output_overlap_details(self, cli_param, previous): def setup_remote_server( self, ctx: click.Context, - name: str = None, - url: str = None, - api_key: str = None, + name: Optional[str] = None, + url: Optional[str] = None, + api_key: Optional[str] = None, insecure: bool = False, - cacert: str = None, - ca_data: str = None, - account_name: str = None, - token: str = None, - secret: str = None, + cacert: Optional[str] = None, + ca_data: Optional[str] = None, + account_name: Optional[str] = None, + token: Optional[str] = None, + secret: Optional[str] = None, ): validation.validate_connection_options( ctx=ctx, diff --git a/rsconnect/bundle.py b/rsconnect/bundle.py index ad7770fe..f6a3b21c 100644 --- a/rsconnect/bundle.py +++ b/rsconnect/bundle.py @@ -2,6 +2,8 @@ Manifest generation and bundling utilities """ +from __future__ import annotations + import hashlib import io import json @@ -11,21 +13,16 @@ import sys import tarfile import tempfile +import typing import re from pprint import pformat from collections import defaultdict from mimetypes import guess_type from pathlib import Path from copy import deepcopy -from typing import List +from typing import Optional, Any import click - -try: - import typing -except ImportError: - typing = None - from os.path import basename, dirname, exists, isdir, join, relpath, splitext, isfile, abspath from .log import logger, VERBOSE @@ -302,14 +299,14 @@ def make_source_manifest( app_mode: AppMode, environment: Environment, entrypoint: str, - quarto_inspection: typing.Dict[str, typing.Any], - image: str = None, - env_management_py: bool = None, - env_management_r: bool = None, -) -> typing.Dict[str, typing.Any]: - manifest = { + quarto_inspection: Optional[dict[str, Any]], + image: Optional[str] = None, + env_management_py: Optional[bool] = None, + env_management_r: Optional[bool] = None, +) -> dict[str, Any]: + manifest: dict[str, Any] = { "version": 1, - } # type: typing.Dict[str, typing.Any] + } # When adding locale, add it early so it is ordered immediately after # version. @@ -847,11 +844,11 @@ def make_api_manifest( entry_point: str, app_mode: AppMode, environment: Environment, - extra_files: typing.List[str], - excludes: typing.List[str], - image: str = None, - env_management_py: bool = None, - env_management_r: bool = None, + extra_files: list[str], + excludes: list[str], + image: Optional[str] = None, + env_management_py: Optional[bool] = None, + env_management_r: Optional[bool] = None, ) -> typing.Tuple[typing.Dict[str, typing.Any], typing.List[str]]: """ Makes a manifest for an API. @@ -1572,12 +1569,11 @@ def which_python(python: typing.Optional[str] = None): def inspect_environment( - python, # type: str - directory, # type: str - force_generate=False, # type: bool - check_output=subprocess.check_output, # type: typing.Callable -): - # type: (...) -> Environment + python: str, + directory: str, + force_generate: bool = False, + check_output: typing.Callable = subprocess.check_output, +) -> Environment: """Run the environment inspector using the specified python binary. Returns a dictionary of information about the environment, @@ -1595,7 +1591,7 @@ def inspect_environment( environment_json = check_output(args, universal_newlines=True) except subprocess.CalledProcessError as e: raise RSConnectException("Error inspecting environment: %s" % e.output) - return MakeEnvironment(**json.loads(environment_json)) # type: ignore + return MakeEnvironment(**json.loads(environment_json)) def get_python_env_info(file_name, python, force_generate=False): diff --git a/rsconnect/environment.py b/rsconnect/environment.py index 75afc1ab..5a4fb174 100644 --- a/rsconnect/environment.py +++ b/rsconnect/environment.py @@ -6,6 +6,8 @@ python -m rsconnect.environment ``` """ +from __future__ import annotations + import collections import datetime import json @@ -14,12 +16,7 @@ import re import subprocess import sys - -try: - import typing - from typing import Optional -except ImportError: - typing = None +from typing import Optional version_re = re.compile(r"\d+\.\d+(\.\d+)?") exec_dir = os.path.dirname(sys.executable) @@ -50,7 +47,7 @@ def MakeEnvironment( python: Optional[str] = None, source: Optional[str] = None, **kwargs, # provides compatibility where we no longer support some older properties -): +) -> Environment: return Environment(contents, error, filename, locale, package_manager, pip, python, source) diff --git a/rsconnect/main.py b/rsconnect/main.py index 356ad9b3..3441e830 100644 --- a/rsconnect/main.py +++ b/rsconnect/main.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import functools import json import os @@ -1403,15 +1405,15 @@ def deploy_app( verbose: int, directory, extra_files, - visibility: typing.Optional[str], - env_vars: typing.Dict[str, str], + visibility: Optional[str], + env_vars: dict[str, str], image: str, disable_env_management: bool, env_management_py: bool, env_management_r: bool, - account: str = None, - token: str = None, - secret: str = None, + account: Optional[str] = None, + token: Optional[str] = None, + secret: Optional[str] = None, no_verify: bool = False, ): set_verbosity(verbose) diff --git a/rsconnect/validation.py b/rsconnect/validation.py index 6a3d9bc2..043e6b61 100644 --- a/rsconnect/validation.py +++ b/rsconnect/validation.py @@ -1,4 +1,7 @@ +from __future__ import annotations + import typing +from typing import Optional import click @@ -35,14 +38,14 @@ def _get_present_options( def validate_connection_options( ctx: click.Context, - url: str, - api_key: str, + url: Optional[str], + api_key: Optional[str], insecure: bool, - cacert: str, - account_name: str, - token: str, - secret: str, - name: str = None, + cacert: Optional[str], + account_name: Optional[str], + token: Optional[str], + secret: Optional[str], + name: Optional[str] = None, ): """ Validates provided Connect or shinyapps.io connection options and returns which target to use given the provided From 1aff19559f9c33d6c5524cb251bb9021c572ab87 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Wed, 14 Feb 2024 12:44:01 -0600 Subject: [PATCH 02/23] Add more type annotations --- rsconnect/api.py | 15 +++++++++------ rsconnect/bundle.py | 26 +++++++++++++------------- rsconnect/certificates.py | 4 +++- rsconnect/main.py | 10 +++++----- rsconnect/validation.py | 6 +++--- 5 files changed, 33 insertions(+), 28 deletions(-) diff --git a/rsconnect/api.py b/rsconnect/api.py index f86bb5e0..867d7d11 100644 --- a/rsconnect/api.py +++ b/rsconnect/api.py @@ -34,6 +34,9 @@ from .bundle import _default_title, fake_module_file_from_directory from .timeouts import get_task_timeout, get_task_timeout_help_message +if typing.TYPE_CHECKING: + import logging + class AbstractRemoteServer: def __init__(self, url: str, remote_name: str): @@ -400,13 +403,13 @@ def __init__( insecure: bool = False, cacert: Optional[str] = None, ca_data: Optional[str] = None, - cookies=None, - account=None, + cookies: Optional[CookieJar] = None, + account: Optional[str] = None, token: Optional[str] = None, secret: Optional[str] = None, timeout: int = 30, - logger=console_logger, - **kwargs + logger: logging.Logger = console_logger, + **kwargs: typing.Any, ) -> None: self.reset() self._d = kwargs @@ -473,13 +476,13 @@ def output_overlap_details(self, cli_param, previous): def setup_remote_server( self, - ctx: click.Context, + ctx: Optional[click.Context], name: Optional[str] = None, url: Optional[str] = None, api_key: Optional[str] = None, insecure: bool = False, cacert: Optional[str] = None, - ca_data: Optional[str] = None, + ca_data: Optional[str | bytes] = None, account_name: Optional[str] = None, token: Optional[str] = None, secret: Optional[str] = None, diff --git a/rsconnect/bundle.py b/rsconnect/bundle.py index f6a3b21c..d67836a8 100644 --- a/rsconnect/bundle.py +++ b/rsconnect/bundle.py @@ -1381,7 +1381,7 @@ def validate_file_is_notebook(file_name): raise RSConnectException("A Jupyter notebook (.ipynb) file is required here.") -def validate_extra_files(directory, extra_files, use_abspath=False): +def validate_extra_files(directory: str, extra_files: typing.Sequence[str], use_abspath: bool = False) -> list[str]: """ If the user specified a list of extra files, validate that they all exist and are beneath the given directory and, if so, return a list of them made relative to that @@ -1391,7 +1391,7 @@ def validate_extra_files(directory, extra_files, use_abspath=False): :param extra_files: the list of extra files to qualify and validate. :return: the extra files qualified by the directory. """ - result = [] + result: list[str] = [] if extra_files: for extra in extra_files: extra_file = relpath(extra, directory) @@ -1425,7 +1425,7 @@ def validate_manifest_file(file_or_directory): re_app_suffix = re.compile(r".+[-_]app\.py$") -def get_default_entrypoint(directory): +def get_default_entrypoint(directory: str): candidates = ["app", "application", "main", "api"] files = set(os.listdir(directory)) @@ -1448,7 +1448,7 @@ def get_default_entrypoint(directory): raise RSConnectException(f"Could not determine default entrypoint file in directory '{directory}'") -def validate_entry_point(entry_point, directory): +def validate_entry_point(entry_point: str | None, directory: str): """ Validates the entry point specified by the user, expanding as necessary. If the user specifies nothing, a module of "app" is assumed. If the user specifies a @@ -1477,7 +1477,7 @@ def _warn_on_ignored_entrypoint(entrypoint): ) -def _warn_on_ignored_manifest(directory): +def _warn_on_ignored_manifest(directory: str): """ Checks for the existence of a file called manifest.json in the given directory. If it's there, a warning noting that it will be ignored will be printed. @@ -1491,7 +1491,7 @@ def _warn_on_ignored_manifest(directory): ) -def _warn_if_no_requirements_file(directory): +def _warn_if_no_requirements_file(directory: str): """ Checks for the existence of a file called requirements.txt in the given directory. If it's not there, a warning will be printed. @@ -1506,7 +1506,7 @@ def _warn_if_no_requirements_file(directory): ) -def _warn_if_environment_directory(directory): +def _warn_if_environment_directory(directory: str): """ Issue a warning if the deployment directory is itself a virtualenv (yikes!). @@ -1520,7 +1520,7 @@ def _warn_if_environment_directory(directory): ) -def _warn_on_ignored_requirements(directory, requirements_file_name): +def _warn_on_ignored_requirements(directory: str, requirements_file_name: str): """ Checks for the existence of a file called manifest.json in the given directory. If it's there, a warning noting that it will be ignored will be printed. @@ -1548,7 +1548,7 @@ def fake_module_file_from_directory(directory: str): return join(directory, app_name + ".py") -def which_python(python: typing.Optional[str] = None): +def which_python(python: Optional[str] = None): """Determines which Python executable to use. If the :param python: is provided, then validation is performed to check if the path is an executable file. If @@ -1594,7 +1594,7 @@ def inspect_environment( return MakeEnvironment(**json.loads(environment_json)) -def get_python_env_info(file_name, python, force_generate=False): +def get_python_env_info(file_name: str, python: str | None, force_generate=False): """ Gathers the python and environment information relating to the specified file with an eye to deploy it. @@ -2064,10 +2064,10 @@ def write_manifest_json(manifest_path, manifest): def create_python_environment( - directory: str = None, + directory: str, force_generate: bool = False, - python: str = None, -): + python: Optional[str] = None, +) -> Environment: module_file = fake_module_file_from_directory(directory) # click.secho(' Deploying %s to server "%s"' % (directory, connect_server.url)) diff --git a/rsconnect/certificates.py b/rsconnect/certificates.py index 72bc7c64..1639b149 100644 --- a/rsconnect/certificates.py +++ b/rsconnect/certificates.py @@ -1,10 +1,12 @@ +from __future__ import annotations + from pathlib import Path BINARY_ENCODED_FILETYPES = [".cer", ".der"] TEXT_ENCODED_FILETYPES = [".ca-bundle", ".crt", ".key", ".pem"] -def read_certificate_file(location: str): +def read_certificate_file(location: str) -> str | bytes: """Reads a certificate file from disk. The file type (suffix) is used to determine the file encoding. diff --git a/rsconnect/main.py b/rsconnect/main.py index 3441e830..a115a740 100644 --- a/rsconnect/main.py +++ b/rsconnect/main.py @@ -1395,16 +1395,16 @@ def deploy_app( api_key: str, insecure: bool, cacert: str, - entrypoint, - exclude, + entrypoint: Optional[str], + exclude: Optional[list[str]], new: bool, app_id: str, title: str, - python, + python: Optional[str], force_generate: bool, verbose: int, - directory, - extra_files, + directory: str, + extra_files: tuple[str, ...] | list[str], visibility: Optional[str], env_vars: dict[str, str], image: str, diff --git a/rsconnect/validation.py b/rsconnect/validation.py index 043e6b61..45577c19 100644 --- a/rsconnect/validation.py +++ b/rsconnect/validation.py @@ -10,7 +10,7 @@ def get_parameter_source_name_from_ctx( var_or_param_name: str, - ctx: click.Context, + ctx: Optional[click.Context], ) -> str: if ctx: varName = var_or_param_name.replace("-", "_") @@ -22,7 +22,7 @@ def get_parameter_source_name_from_ctx( def _get_present_options( options: typing.Dict[str, typing.Optional[typing.Any]], - ctx: click.Context, + ctx: Optional[click.Context], ) -> typing.List[str]: result: typing.List[str] = [] for k, v in options.items(): @@ -37,7 +37,7 @@ def _get_present_options( def validate_connection_options( - ctx: click.Context, + ctx: Optional[click.Context], url: Optional[str], api_key: Optional[str], insecure: bool, From d9c218933a51e297c09bdfc091b45c67673fef15 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Wed, 14 Feb 2024 13:02:56 -0600 Subject: [PATCH 03/23] More type annotations --- rsconnect/api.py | 22 +++++++++++----------- rsconnect/main.py | 35 +++++++++++++++++------------------ 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/rsconnect/api.py b/rsconnect/api.py index 867d7d11..be21ae29 100644 --- a/rsconnect/api.py +++ b/rsconnect/api.py @@ -667,7 +667,7 @@ def validate_rstudio_server( raise RSConnectException("Failed to verify with {} ({}).".format(server.remote_name, exc)) @cls_logged("Making bundle ...") - def make_bundle(self, func: Callable, *args, **kwargs): + def make_bundle(self, func: Callable[..., object], *args: object, **kwargs: object): path = ( self.get("path", **kwargs) or self.get("file", **kwargs) @@ -716,14 +716,14 @@ def upload_rstudio_bundle(self, prepare_deploy_result, bundle_size: int, content @cls_logged("Deploying bundle ...") def deploy_bundle( self, - app_id: typing.Union[str, int] = None, - deployment_name: str = None, - title: str = None, + app_id: Optional[str | int] = None, + deployment_name: Optional[str] = None, + title: Optional[str] = None, title_is_default: bool = False, - bundle: IO = None, - env_vars=None, - app_mode=None, - visibility=None, + bundle: Optional[IO[str] | IO[bytes]] = None, + env_vars: Optional[dict[str, str]]=None, + app_mode: Optional[AppMode]=None, + visibility: Optional[str]=None, ): app_id = app_id or self.get("app_id") deployment_name = deployment_name or self.get("deployment_name") @@ -823,7 +823,7 @@ def emit_task_log( return self @cls_logged("Saving deployed information...") - def save_deployed_info(self, *args, **kwargs): + def save_deployed_info(self, *args: object, **kwargs: object): app_store = self.get("app_store", *args, **kwargs) path = ( self.get("path", **kwargs) @@ -847,14 +847,14 @@ def save_deployed_info(self, *args, **kwargs): return self @cls_logged("Verifying deployed content...") - def verify_deployment(self, *args, **kwargs): + def verify_deployment(self, *args: object, **kwargs: object): if isinstance(self.remote_server, RSConnectServer): deployed_info = self.get("deployed_info", *args, **kwargs) app_guid = deployed_info["app_guid"] self.client.app_access(app_guid) @cls_logged("Validating app mode...") - def validate_app_mode(self, *args, **kwargs): + def validate_app_mode(self, *args: object, **kwargs: object): path = ( self.get("path", **kwargs) or self.get("file", **kwargs) diff --git a/rsconnect/main.py b/rsconnect/main.py index a115a740..3a7e2e2b 100644 --- a/rsconnect/main.py +++ b/rsconnect/main.py @@ -1452,25 +1452,24 @@ def deploy_app( **extra_args, ) - ( - ce.validate_server() - .validate_app_mode(app_mode=app_mode) - .make_bundle( - make_api_bundle, - directory, - entrypoint, - app_mode, - environment, - extra_files, - exclude, - image=image, - env_management_py=env_management_py, - env_management_r=env_management_r, - ) - .deploy_bundle() - .save_deployed_info() - .emit_task_log() + ce.validate_server() + ce.validate_app_mode(app_mode=app_mode) + ce.make_bundle( + make_api_bundle, + directory, + entrypoint, + app_mode, + environment, + extra_files, + exclude, + image=image, + env_management_py=env_management_py, + env_management_r=env_management_r, ) + ce.deploy_bundle() + ce.save_deployed_info() + ce.emit_task_log() + if not no_verify: ce.verify_deployment() From 275ffa0561bc28fabba0ecfe73edb9f57fbf4f35 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Wed, 14 Feb 2024 15:28:56 -0600 Subject: [PATCH 04/23] More tye annotations --- rsconnect/environment.py | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/rsconnect/environment.py b/rsconnect/environment.py index 5a4fb174..c6d7404d 100644 --- a/rsconnect/environment.py +++ b/rsconnect/environment.py @@ -8,7 +8,6 @@ """ from __future__ import annotations -import collections import datetime import json import locale @@ -16,25 +15,21 @@ import re import subprocess import sys -from typing import Optional +from typing import NamedTuple, Optional version_re = re.compile(r"\d+\.\d+(\.\d+)?") exec_dir = os.path.dirname(sys.executable) -Environment = collections.namedtuple( - "Environment", - ( - "contents", - "error", - "filename", - "locale", - "package_manager", - "pip", - "python", - "source", - ), -) +class Environment(NamedTuple): + contents: Optional[str] + error: Optional[str] + filename: Optional[str] + locale: Optional[str] + package_manager: Optional[str] + pip: Optional[str] + python: Optional[str] + source: Optional[str] def MakeEnvironment( @@ -46,7 +41,7 @@ def MakeEnvironment( pip: Optional[str] = None, python: Optional[str] = None, source: Optional[str] = None, - **kwargs, # provides compatibility where we no longer support some older properties + **kwargs: object, # provides compatibility where we no longer support some older properties ) -> Environment: return Environment(contents, error, filename, locale, package_manager, pip, python, source) @@ -55,8 +50,7 @@ class EnvironmentException(Exception): pass -def detect_environment(dirname, force_generate=False): - # type: (str, bool) -> Environment +def detect_environment(dirname: str, force_generate: bool = False) -> Environment: """Determine the python dependencies in the environment. `pip freeze` will be used to introspect the environment. @@ -80,8 +74,7 @@ def detect_environment(dirname, force_generate=False): return MakeEnvironment(**result) -def get_python_version(environment): - # type: (Environment) -> str +def get_python_version(environment: Environment) -> str: v = sys.version_info return "%d.%d.%d" % (v[0], v[1], v[2]) From 8384e1bb1b0850aed13329824cffe402d00bc0a2 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Wed, 14 Feb 2024 22:24:59 -0600 Subject: [PATCH 05/23] Update starlette version in requirements if necessary --- rsconnect/main.py | 42 +++++++++- rsconnect/utils_package.py | 168 +++++++++++++++++++++++++++++++++++++ tests/test_version.py | 104 +++++++++++++++++++++++ 3 files changed, 313 insertions(+), 1 deletion(-) create mode 100644 rsconnect/utils_package.py create mode 100644 tests/test_version.py diff --git a/rsconnect/main.py b/rsconnect/main.py index 3a7e2e2b..59061266 100644 --- a/rsconnect/main.py +++ b/rsconnect/main.py @@ -3,6 +3,7 @@ import functools import json import os +import re import sys import traceback import typing @@ -10,7 +11,7 @@ import click from os.path import abspath, dirname, exists, isdir, join from functools import wraps -from typing import Optional +from typing import Literal, Optional, cast from rsconnect.certificates import read_certificate_file @@ -83,6 +84,7 @@ parse_client_response, ) from .shiny_express import escape_to_var_name, is_express_app +from .utils_package import compare_semvers, parse_requirements_txt, find_package_info server_store = ServerStore() future_enabled = False @@ -1453,6 +1455,44 @@ def deploy_app( ) ce.validate_server() + + # TODO: Extract to a function + connect_version_string = cast(str, ce.server_details["connect"]) # type: ignore + if app_mode == AppModes.PYTHON_SHINY and compare_semvers(connect_version_string, "2024.01.0") == -1: + print(f"Connect server version: {connect_version_string}") + + requirements_txt = cast(str, environment.contents) + + reqs = parse_requirements_txt(requirements_txt) + starlette_req = find_package_info("starlette", reqs) + + if starlette_req is None: + print("Adding starlette<0.35.0 to requirements") + environment = environment._replace(contents=requirements_txt.rstrip("\n") + "\nstarlette<0.35.0\n") + + elif len(starlette_req.specs) == 0: + print("Adding starlette<0.35.0 to requirements") + # TODO: Need safer modification of requirements.txt + environment = environment._replace( + contents=re.sub(r"\nstarlette.*?\n", "\nstarlette<0.35.0\n", requirements_txt) + ) + + elif len(starlette_req.specs) == 1 and starlette_req.specs[0].operator == "==": + if compare_semvers(starlette_req.specs[0].version, "0.35.0") >= 0: + print("Starlette version is 0.35.0 or greater. Setting to <0.35.0.") + + environment = environment._replace( + contents=re.sub(r"\nstarlette.*?\n", "\nstarlette<0.35.0\n", requirements_txt) + ) + else: + # If more complex version spec (that doesn't use ==), do nothing. + pass + + # If didn't ask for specific version of starlette, add starlette<0.35.0. + # If pinned version is <0.35.0, do nothing. + # If pinned version is >=0.35.0, change the version and emit warning + # If more complex version spec (that doesn't use ==), do nothing. + ce.validate_app_mode(app_mode=app_mode) ce.make_bundle( make_api_bundle, diff --git a/rsconnect/utils_package.py b/rsconnect/utils_package.py new file mode 100644 index 00000000..145bd0c0 --- /dev/null +++ b/rsconnect/utils_package.py @@ -0,0 +1,168 @@ +""" +Utility functions related to packages and versions. +""" + +from __future__ import annotations + +import re +from typing import Literal, NamedTuple, cast + +import semver + +ComparisonOperator = Literal[">=", "<=", ">", "<", "==", "!="] + + +class VersionInfo(NamedTuple): + operator: ComparisonOperator + version: str + + +class PackageInfo(NamedTuple): + name: str + specs: list[VersionInfo] + + +def compare_semvers(version1: str, version2: str) -> Literal[-1, 0, 1]: + """ + This is a wrapper for semver.VersionInfo.compare(), but it can handle some version + strings that are not valid semver strings. Specifically, it can handle strings like + "2024.01.0", which have a leading '0' in a version part. The leading zero in "01" + makes it an invalid semver string. + + Note that semvers always have three parts, so "0.12" is not a valid semver. This + function can be safely used for comparing Connect version numbers, but not for + comparing Python package numbers, because they may not have exactly three parts. + + This function is better for comparing Connect version numbers than + compare_package_versions(), because the latter doesn't handle the prerelease and + build components of the version string. For "2024.01.0-dev+20-abcd", the prerelease + and build components are "dev" and "20-abcd", respectively. + + :return: -1 if version1 < version2, 0 if version1 == version2, or 1 if version1 > + version2. + """ + + version1 = _remove_leading_zeros(version1) + version2 = _remove_leading_zeros(version2) + + return semver.VersionInfo.parse(version1).compare(version2) # type: ignore + + +def _remove_leading_zeros(version: str) -> str: + """ + Given a version string like "2024.01.0-dev+20-abcd", remove any leading zeros in a + version part, so that the result is "2024.1.0-dev+20-abcd". + """ + parts = version.split(".") + parts = [re.sub(r"^0+(\d)", "\\1", x) for x in parts] + return ".".join(parts) + + +def compare_package_versions(version1: str, version2: str) -> Literal[-1, 0, 1]: + """ + Compare two package versions. This function is similar to compare_semvers(), but it + can accept version numbers with any number of parts (instead of exactly three). It + is used to compare package versions, which may not be valid semver strings. + """ + parts1 = [int(x) for x in version1.split(".")] + parts2 = [int(x) for x in version2.split(".")] + + max_length = max(len(parts1), len(parts2)) + parts1 += [0] * (max_length - len(parts1)) + parts2 += [0] * (max_length - len(parts2)) + + for part1, part2 in zip(parts1, parts2): + if part1 > part2: + return 1 + elif part1 < part2: + return -1 + + return 0 + + +def parse_requirements_txt(requirements_txt: str) -> list[PackageInfo]: + """ + Given a string of requirements (like the contents of a requirements.txt file), find + the version of the package that is installed. This is done by finding the line that + starts with the package name, and then extracting the version from that line. + + Note that this function does not handle cases where there are multipler version + requirements for a package, as in "pkg>=1.0,<=2.0". It only handles simple cases, + like "pkg==1.0" or "pkg>=1.0". + + :param requirements_txt: The contents of a requirements.txt file. + :param package: The name of the package to find. + + :return: The version of the package, or None if the package is not found. + """ + # Note that this parser is not perfect. If we want to perfectly parse a + # requirements.txt file, we could use the requirements-parser package, but that + # would introduce another dependency. + + package_info: list[PackageInfo] = [] + + for line in requirements_txt.split("\n"): + # Skip blank lines or comment lines + if re.match(r"^\s*$", line) or re.match(r"^\s*#", line): + continue + + pkg_match = re.match(r"^([A-Z0-9][A-Z0-9][A-Z0-9._-]*[A-Z0-9])(.*)", line, flags=re.IGNORECASE) + if pkg_match is None: + continue + + # If the line is "foo>=1.0", this is "foo". + pkg_name = cast(str, pkg_match.group(1)) + + # If the line is "foo>=1.0", this is ">=1.0". + pkg_version_string = cast(str, pkg_match.group(2)).strip() + # Remove comment from version string, if present. + pkg_version_string = re.sub(r"#.*$", "", pkg_version_string).strip() + pkg_version_specs = _parse_version_specs(pkg_version_string) + + package_info.append(PackageInfo(name=pkg_name, specs=pkg_version_specs)) + + return package_info + + +def _parse_version_specs(version_specs: str) -> list[VersionInfo]: + """ + Parse a version spec string like ">=1.0,<=2.0" into a list of tuples like [(">=", + "1.0"), ("<=", "2.0")]. If there is a semicolon, as in "==1.0; + python_version<'3.8'", then the semicolon and everything after it is ignored. + If it finds a spec string that it is unable to parse, it will ignore that string. + """ + + version_specs = version_specs.split(";")[0].strip() + if version_specs.strip() == "": + return [] + version_spec_strings = [x.strip() for x in version_specs.split(",")] + + version_infos: list[VersionInfo] = [] + for version_spec_string in version_spec_strings: + spec = _parse_version_spec(version_spec_string) + if spec is not None: + version_infos.append(spec) + + return version_infos + + +def _parse_version_spec(version_spec: str) -> VersionInfo | None: + """ + Parse a version spec string like ">=1.0" into a tuple like (">=", "1.0"). If it is + unable to parse the string, return None. + """ + for op in (">=", "<=", ">", "<", "==", "!="): + if version_spec.startswith(op): + return VersionInfo(op, version_spec[len(op) :].strip()) + return None + + +def find_package_info(package: str, requirements: list[PackageInfo]) -> PackageInfo | None: + """ + Given a package name and a list of PackageInfo objects, find the PackageInfo object + that corresponds to the package name. If the package is not found, return None. + """ + for pkg_info in requirements: + if pkg_info.name == package: + return pkg_info + return None diff --git a/tests/test_version.py b/tests/test_version.py new file mode 100644 index 00000000..de1ecaca --- /dev/null +++ b/tests/test_version.py @@ -0,0 +1,104 @@ +from textwrap import dedent +from rsconnect.utils_package import ( + _remove_leading_zeros, + compare_package_versions, + compare_semvers, + parse_requirements_txt, +) + + +def test_remove_leading_zeros(): + assert _remove_leading_zeros("2024.01.02") == "2024.1.2" + assert _remove_leading_zeros("2024.001.020") == "2024.1.20" + + # Shouldn't remove leading zero from prerelease ("dev") or build ("020-abcd") + # strings. + assert _remove_leading_zeros("2020.01.0-dev+020-abcd") == "2020.1.0-dev+020-abcd" + + +def test_compare_semvers(): + # Different combinations of leading zeros + assert compare_semvers("2023.01.0", "2023.02.0") == -1 + assert compare_semvers("2023.1.0", "2023.02.0") == -1 + assert compare_semvers("2023.01.0", "2023.2.0") == -1 + + assert compare_semvers("2023.01.0", "2023.01.0") == 0 + assert compare_semvers("2023.1.0", "2023.01.00") == 0 + assert compare_semvers("2023.01.0", "2023.1.0") == 0 + + assert compare_semvers("2023.01.3", "2023.1.3") == 0 + assert compare_semvers("2023.01.03", "2023.1.03") == 0 + assert compare_semvers("2023.01.3", "2023.01.3") == 0 + + assert compare_semvers("2023.02.0", "2023.01.0") == 1 + assert compare_semvers("2023.2.0", "2023.01.0") == 1 + assert compare_semvers("2023.02.0", "2023.1.0") == 1 + + assert compare_semvers("2023.01.0", "2023.1.01") == -1 + assert compare_semvers("2024.01.0", "2023.2.03") == 1 + + +def test_compare_package_versions(): + assert compare_package_versions("3.01.0", "3.02") == -1 + assert compare_package_versions("3.1.0", "3.02.0") == -1 + assert compare_package_versions("0.01.0", "0.2.0") == -1 + assert compare_package_versions("0.01.5", "0.2.0") == -1 + + assert compare_package_versions("3.01.0", "3.1") == 0 + assert compare_package_versions("3.1.0", "3.01.0") == 0 + assert compare_package_versions("0.01.0", "0.1.0") == 0 + assert compare_package_versions("0.01", "0.1.0") == 0 + assert compare_package_versions("0.01.0.0", "0.1") == 0 + + assert compare_package_versions("3.02", "3.01.0") == 1 + assert compare_package_versions("3.02.0", "3.1.0") == 1 + assert compare_package_versions("0.2.0", "0.01.0") == 1 + assert compare_package_versions("0.2.0", "0.01.5") == 1 + + +def test_parse_requirements(): + res = parse_requirements_txt( + dedent( + """ + pkga==1.0 + pkgb>=2.0 # Comment + pkgc>1.0,<=3.0 + pkgd >1.0, <=3.0 + pkge >1.0,<=3.0 ; python_version<'3.8' + pkgf; python_version<'3.8' + pkgg + + # Comment line + # Another comment line + pkg-h1 # Comment + """ + ) + ) + assert res == [ + ("pkga", [("==", "1.0")]), + ("pkgb", [(">=", "2.0")]), + ("pkgc", [(">", "1.0"), ("<=", "3.0")]), + ("pkgd", [(">", "1.0"), ("<=", "3.0")]), + ("pkge", [(">", "1.0"), ("<=", "3.0")]), + ("pkgf", []), + ("pkgg", []), + ("pkg-h1", []), + ] + + # Malformed version specs should be ignored + res = parse_requirements_txt( + dedent( + """ + pkga==1.0 + pkgb!!2.0 + pkgc>2.0,[=3.0 + """ + ) + ) + assert res == [ + ("pkga", [("==", "1.0")]), + ("pkgb", []), + ("pkgc", [(">", "2.0")]), + ] + + print(res) From f732de82d342bda94b948c4b843c14df2456eb31 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Wed, 14 Feb 2024 23:37:52 -0600 Subject: [PATCH 06/23] Update docstring --- rsconnect/utils_package.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/rsconnect/utils_package.py b/rsconnect/utils_package.py index 145bd0c0..7332e700 100644 --- a/rsconnect/utils_package.py +++ b/rsconnect/utils_package.py @@ -86,14 +86,10 @@ def parse_requirements_txt(requirements_txt: str) -> list[PackageInfo]: the version of the package that is installed. This is done by finding the line that starts with the package name, and then extracting the version from that line. - Note that this function does not handle cases where there are multipler version - requirements for a package, as in "pkg>=1.0,<=2.0". It only handles simple cases, - like "pkg==1.0" or "pkg>=1.0". - :param requirements_txt: The contents of a requirements.txt file. :param package: The name of the package to find. - :return: The version of the package, or None if the package is not found. + :return: A list of PackageInfo objects. """ # Note that this parser is not perfect. If we want to perfectly parse a # requirements.txt file, we could use the requirements-parser package, but that From e1d81f8a0e6196ed2787990f6a23cfaaa8a837cf Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Wed, 14 Feb 2024 23:39:03 -0600 Subject: [PATCH 07/23] Cleaner method for modifying requirements.txt --- rsconnect/main.py | 41 +++++++++++++++++++------------------- rsconnect/utils_package.py | 19 ++++++++++++++++++ tests/test_version.py | 28 ++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 20 deletions(-) diff --git a/rsconnect/main.py b/rsconnect/main.py index 59061266..e30b8810 100644 --- a/rsconnect/main.py +++ b/rsconnect/main.py @@ -84,7 +84,7 @@ parse_client_response, ) from .shiny_express import escape_to_var_name, is_express_app -from .utils_package import compare_semvers, parse_requirements_txt, find_package_info +from .utils_package import compare_semvers, parse_requirements_txt, find_package_info, replace_requirement server_store = ServerStore() future_enabled = False @@ -1454,8 +1454,6 @@ def deploy_app( **extra_args, ) - ce.validate_server() - # TODO: Extract to a function connect_version_string = cast(str, ce.server_details["connect"]) # type: ignore if app_mode == AppModes.PYTHON_SHINY and compare_semvers(connect_version_string, "2024.01.0") == -1: @@ -1466,33 +1464,36 @@ def deploy_app( reqs = parse_requirements_txt(requirements_txt) starlette_req = find_package_info("starlette", reqs) + # If didn't ask for specific version of starlette, add starlette<0.35.0. + # If pinned version is <0.35.0, do nothing. + # If pinned version is >=0.35.0, change the version and emit warning + # If more complex version spec (that doesn't use ==), do nothing. + if starlette_req is None: - print("Adding starlette<0.35.0 to requirements") + # starlette is not listed in requirements. + # Add starlette<0.35.0 to requirements. environment = environment._replace(contents=requirements_txt.rstrip("\n") + "\nstarlette<0.35.0\n") elif len(starlette_req.specs) == 0: - print("Adding starlette<0.35.0 to requirements") - # TODO: Need safer modification of requirements.txt - environment = environment._replace( - contents=re.sub(r"\nstarlette.*?\n", "\nstarlette<0.35.0\n", requirements_txt) - ) + # starlette is in requirements, but without a version specification. + # Replace it with starlette<0.35.0. + requirements_txt = replace_requirement("starlette", "starlette<0.35.0", requirements_txt) + environment = environment._replace(contents=requirements_txt) elif len(starlette_req.specs) == 1 and starlette_req.specs[0].operator == "==": if compare_semvers(starlette_req.specs[0].version, "0.35.0") >= 0: - print("Starlette version is 0.35.0 or greater. Setting to <0.35.0.") - - environment = environment._replace( - contents=re.sub(r"\nstarlette.*?\n", "\nstarlette<0.35.0\n", requirements_txt) - ) + # starlette is in requirements.txt, but with a version spec that is + # not compatible with this version of Connect. + print("Starlette version is 0.35.0 or greater, but this version of Connect requires starlette<0.35.0. Setting to <0.35.0.") + requirements_txt = replace_requirement("starlette", "starlette<0.35.0", requirements_txt) + environment = environment._replace(contents=requirements_txt) else: - # If more complex version spec (that doesn't use ==), do nothing. + # If more complex version spec (e.g., it uses something other than == or + # has multiple specs), do nothing, because this is an advanced user that + # is doing something complicated. pass - # If didn't ask for specific version of starlette, add starlette<0.35.0. - # If pinned version is <0.35.0, do nothing. - # If pinned version is >=0.35.0, change the version and emit warning - # If more complex version spec (that doesn't use ==), do nothing. - + ce.validate_server() ce.validate_app_mode(app_mode=app_mode) ce.make_bundle( make_api_bundle, diff --git a/rsconnect/utils_package.py b/rsconnect/utils_package.py index 7332e700..936a25e9 100644 --- a/rsconnect/utils_package.py +++ b/rsconnect/utils_package.py @@ -162,3 +162,22 @@ def find_package_info(package: str, requirements: list[PackageInfo]) -> PackageI if pkg_info.name == package: return pkg_info return None + + +def replace_requirement(package_name: str, replacement: str, requirements_txt: str) -> str: + """ + Given a requirements.txt file and a package name, replace the line of + requirements.txt for that package, with the replacement string, and return the + modified requirements.txt. + + Note that package_name is a regular expression, so if the target package name + contains a ".", it should be escaped, as in "foo\\.bar". + """ + lines = requirements_txt.split("\n") + new_lines: list[str] = [] + + for line in lines: + new_line = re.sub(f"^{package_name}([^a-zA-Z0-9._-].*?)?$", replacement, line) + new_lines.append(new_line) + + return "\n".join(new_lines) diff --git a/tests/test_version.py b/tests/test_version.py index de1ecaca..df30d38b 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -4,6 +4,7 @@ compare_package_versions, compare_semvers, parse_requirements_txt, + replace_requirement, ) @@ -102,3 +103,30 @@ def test_parse_requirements(): ] print(res) + + +def test_replace_requirement(): + x = replace_requirement( + "starlette", + "REPLACED", + dedent( + """ + abcd + starlette + starlette==1.0 + starlette-foo + starlette0 + starlette + """ + ), + ) + assert x == dedent( + """ + abcd + REPLACED + REPLACED + starlette-foo + starlette0 + REPLACED + """ + ) From e4eae56abba29a39c39c1751ebc4da75d3f6d410 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Thu, 15 Feb 2024 00:00:42 -0600 Subject: [PATCH 08/23] Extract fix_starlette_version function --- rsconnect/main.py | 53 +++++++------------------ rsconnect/utils_package.py | 80 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 90 insertions(+), 43 deletions(-) diff --git a/rsconnect/main.py b/rsconnect/main.py index e30b8810..b3340459 100644 --- a/rsconnect/main.py +++ b/rsconnect/main.py @@ -84,7 +84,13 @@ parse_client_response, ) from .shiny_express import escape_to_var_name, is_express_app -from .utils_package import compare_semvers, parse_requirements_txt, find_package_info, replace_requirement +from .utils_package import ( + compare_semvers, + find_package_info, + fix_starlette_requirements, + parse_requirements_txt, + replace_requirement, +) server_store = ServerStore() future_enabled = False @@ -1454,44 +1460,13 @@ def deploy_app( **extra_args, ) - # TODO: Extract to a function - connect_version_string = cast(str, ce.server_details["connect"]) # type: ignore - if app_mode == AppModes.PYTHON_SHINY and compare_semvers(connect_version_string, "2024.01.0") == -1: - print(f"Connect server version: {connect_version_string}") - - requirements_txt = cast(str, environment.contents) - - reqs = parse_requirements_txt(requirements_txt) - starlette_req = find_package_info("starlette", reqs) - - # If didn't ask for specific version of starlette, add starlette<0.35.0. - # If pinned version is <0.35.0, do nothing. - # If pinned version is >=0.35.0, change the version and emit warning - # If more complex version spec (that doesn't use ==), do nothing. - - if starlette_req is None: - # starlette is not listed in requirements. - # Add starlette<0.35.0 to requirements. - environment = environment._replace(contents=requirements_txt.rstrip("\n") + "\nstarlette<0.35.0\n") - - elif len(starlette_req.specs) == 0: - # starlette is in requirements, but without a version specification. - # Replace it with starlette<0.35.0. - requirements_txt = replace_requirement("starlette", "starlette<0.35.0", requirements_txt) - environment = environment._replace(contents=requirements_txt) - - elif len(starlette_req.specs) == 1 and starlette_req.specs[0].operator == "==": - if compare_semvers(starlette_req.specs[0].version, "0.35.0") >= 0: - # starlette is in requirements.txt, but with a version spec that is - # not compatible with this version of Connect. - print("Starlette version is 0.35.0 or greater, but this version of Connect requires starlette<0.35.0. Setting to <0.35.0.") - requirements_txt = replace_requirement("starlette", "starlette<0.35.0", requirements_txt) - environment = environment._replace(contents=requirements_txt) - else: - # If more complex version spec (e.g., it uses something other than == or - # has multiple specs), do nothing, because this is an advanced user that - # is doing something complicated. - pass + # Update the starlette version if needed. After all users are on Connect + # 2024.01.1 or later, this can be removed. + environment = fix_starlette_requirements( + environment=environment, + app_mode=app_mode, + connect_version_string=cast(str, ce.server_details["connect"]), # type: ignore + ) ce.validate_server() ce.validate_app_mode(app_mode=app_mode) diff --git a/rsconnect/utils_package.py b/rsconnect/utils_package.py index 936a25e9..0e5e6a02 100644 --- a/rsconnect/utils_package.py +++ b/rsconnect/utils_package.py @@ -9,6 +9,9 @@ import semver +from .environment import Environment +from .models import AppMode, AppModes + ComparisonOperator = Literal[">=", "<=", ">", "<", "==", "!="] @@ -24,14 +27,18 @@ class PackageInfo(NamedTuple): def compare_semvers(version1: str, version2: str) -> Literal[-1, 0, 1]: """ + Compare semver-like version numbers. This should be used for comparing Connect + versions, but not Python package versions. + This is a wrapper for semver.VersionInfo.compare(), but it can handle some version - strings that are not valid semver strings. Specifically, it can handle strings like - "2024.01.0", which have a leading '0' in a version part. The leading zero in "01" - makes it an invalid semver string. + strings that are not strictly valid semver strings. Specifically, it can handle + strings like "2024.01.0", which have a leading '0' in a version part. The leading + zero in "01" makes it an invalid semver string. Note that semvers always have three parts, so "0.12" is not a valid semver. This function can be safely used for comparing Connect version numbers, but not for - comparing Python package numbers, because they may not have exactly three parts. + comparing Python package numbers, because the latter may not have exactly three + parts. This function is better for comparing Connect version numbers than compare_package_versions(), because the latter doesn't handle the prerelease and @@ -181,3 +188,68 @@ def replace_requirement(package_name: str, replacement: str, requirements_txt: s new_lines.append(new_line) return "\n".join(new_lines) + + +def fix_starlette_requirements( + environment: Environment, + app_mode: AppMode, + connect_version_string: str, +) -> Environment: + """ + Ensure that the starlette version in an Environment is compatible with the Connect + server. + + If the app mode is PYTHON_SHINY and the Connect server version is less than + 2024.01.0, then make sure the starlette version is less than 0.35.0, due to an + incompatibility with between older Connect<2024.01.0 and starlette>=0.35.0. + + After all users are on Connect 2024.01.0 or later, this function can be removed. + + For more information, see https://github.com/posit-dev/py-shiny/issues/1114 + + :return: Either a modified environment (if changes were needed), or the original + environment. + """ + if not (app_mode == AppModes.PYTHON_SHINY and compare_semvers(connect_version_string, "2024.01.0") == -1): + return environment + + print(f"Connect server version: {connect_version_string}") + + requirements_txt = cast(str, environment.contents) + + reqs = parse_requirements_txt(requirements_txt) + starlette_req = find_package_info("starlette", reqs) + + # If didn't ask for specific version of starlette, add starlette<0.35.0. + # If pinned version is <0.35.0, do nothing. + # If pinned version is >=0.35.0, change the version and emit warning + # If more complex version spec (that doesn't use ==), do nothing. + + if starlette_req is None: + # starlette is not listed in requirements. + # Add starlette<0.35.0 to requirements. + requirements_txt = requirements_txt.rstrip("\n") + "\nstarlette<0.35.0\n" + environment = environment._replace(contents=requirements_txt) + + elif len(starlette_req.specs) == 0: + # starlette is in requirements, but without a version specification. + # Replace it with starlette<0.35.0. + requirements_txt = replace_requirement("starlette", "starlette<0.35.0", requirements_txt) + environment = environment._replace(contents=requirements_txt) + + elif len(starlette_req.specs) == 1 and starlette_req.specs[0].operator == "==": + if compare_semvers(starlette_req.specs[0].version, "0.35.0") >= 0: + # starlette is in requirements.txt, but with a version spec that is + # not compatible with this version of Connect. + print( + "Starlette version is 0.35.0 or greater, but this version of Connect requires starlette<0.35.0. Setting to <0.35.0." + ) + requirements_txt = replace_requirement("starlette", "starlette<0.35.0", requirements_txt) + environment = environment._replace(contents=requirements_txt) + else: + # If more complex version spec (e.g., it uses something other than == or + # has multiple specs), do nothing, because this is an advanced user that + # is doing something complicated. + pass + + return environment \ No newline at end of file From b9c438a3fede595b9a1bd4046f568e69a5a59cb8 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Thu, 15 Feb 2024 00:01:42 -0600 Subject: [PATCH 09/23] Ignore type for kwargs --- rsconnect/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rsconnect/main.py b/rsconnect/main.py index b3340459..4c007257 100644 --- a/rsconnect/main.py +++ b/rsconnect/main.py @@ -1457,7 +1457,7 @@ def deploy_app( account=account, token=token, secret=secret, - **extra_args, + **extra_args, # type: ignore ) # Update the starlette version if needed. After all users are on Connect From 4ee74f7c5a9ec45484864c6dbd52c3efc520f339 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Thu, 15 Feb 2024 00:11:54 -0600 Subject: [PATCH 10/23] Rename test_version.py to test_utils_package.py --- tests/{test_version.py => test_utils_package.py} | 2 -- 1 file changed, 2 deletions(-) rename tests/{test_version.py => test_utils_package.py} (99%) diff --git a/tests/test_version.py b/tests/test_utils_package.py similarity index 99% rename from tests/test_version.py rename to tests/test_utils_package.py index df30d38b..805ae70b 100644 --- a/tests/test_version.py +++ b/tests/test_utils_package.py @@ -102,8 +102,6 @@ def test_parse_requirements(): ("pkgc", [(">", "2.0")]), ] - print(res) - def test_replace_requirement(): x = replace_requirement( From 1d23df22773286ca470cc30f237c79a55d468e58 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Thu, 15 Feb 2024 00:18:32 -0600 Subject: [PATCH 11/23] Update changelog --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e55ff00c..a12fab48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Pending Next Release + +### Changed +- When deploying Shiny for Python applications on servers using a version of + Connect prior to 2024.01.0, there is an incompatibility with + `starlette>=0.35.0`. When deploying to these servers, the starlette version + is now automatically set to `starlette<0.35.0`. + ## [1.22.0] - 2024-01-23 ### Added From 39ad72a0467a8736731a46699071c0b5dc320fcf Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Thu, 15 Feb 2024 00:29:42 -0600 Subject: [PATCH 12/23] Linting fixes --- rsconnect/api.py | 6 +++--- rsconnect/bundle.py | 2 +- rsconnect/main.py | 13 +++---------- rsconnect/utils_package.py | 5 +++-- 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/rsconnect/api.py b/rsconnect/api.py index be21ae29..3642471a 100644 --- a/rsconnect/api.py +++ b/rsconnect/api.py @@ -721,9 +721,9 @@ def deploy_bundle( title: Optional[str] = None, title_is_default: bool = False, bundle: Optional[IO[str] | IO[bytes]] = None, - env_vars: Optional[dict[str, str]]=None, - app_mode: Optional[AppMode]=None, - visibility: Optional[str]=None, + env_vars: Optional[dict[str, str]] = None, + app_mode: Optional[AppMode] = None, + visibility: Optional[str] = None, ): app_id = app_id or self.get("app_id") deployment_name = deployment_name or self.get("deployment_name") diff --git a/rsconnect/bundle.py b/rsconnect/bundle.py index d67836a8..2cc9c587 100644 --- a/rsconnect/bundle.py +++ b/rsconnect/bundle.py @@ -1060,7 +1060,7 @@ def infer_entrypoint(path, mimetype): return candidates.pop() if len(candidates) == 1 else None -def infer_entrypoint_candidates(path, mimetype) -> List: +def infer_entrypoint_candidates(path, mimetype) -> list[str]: if not path: return [] if isfile(path): diff --git a/rsconnect/main.py b/rsconnect/main.py index 4c007257..b9b5b143 100644 --- a/rsconnect/main.py +++ b/rsconnect/main.py @@ -3,7 +3,6 @@ import functools import json import os -import re import sys import traceback import typing @@ -11,7 +10,7 @@ import click from os.path import abspath, dirname, exists, isdir, join from functools import wraps -from typing import Literal, Optional, cast +from typing import Optional, cast from rsconnect.certificates import read_certificate_file @@ -84,13 +83,7 @@ parse_client_response, ) from .shiny_express import escape_to_var_name, is_express_app -from .utils_package import ( - compare_semvers, - find_package_info, - fix_starlette_requirements, - parse_requirements_txt, - replace_requirement, -) +from .utils_package import fix_starlette_requirements server_store = ServerStore() future_enabled = False @@ -1457,7 +1450,7 @@ def deploy_app( account=account, token=token, secret=secret, - **extra_args, # type: ignore + **extra_args, # type: ignore ) # Update the starlette version if needed. After all users are on Connect diff --git a/rsconnect/utils_package.py b/rsconnect/utils_package.py index 0e5e6a02..5d639a02 100644 --- a/rsconnect/utils_package.py +++ b/rsconnect/utils_package.py @@ -242,7 +242,8 @@ def fix_starlette_requirements( # starlette is in requirements.txt, but with a version spec that is # not compatible with this version of Connect. print( - "Starlette version is 0.35.0 or greater, but this version of Connect requires starlette<0.35.0. Setting to <0.35.0." + "Starlette version is 0.35.0 or greater, but this version of Connect " + "requires starlette<0.35.0. Setting to <0.35.0." ) requirements_txt = replace_requirement("starlette", "starlette<0.35.0", requirements_txt) environment = environment._replace(contents=requirements_txt) @@ -252,4 +253,4 @@ def fix_starlette_requirements( # is doing something complicated. pass - return environment \ No newline at end of file + return environment From 3c399156875d1746b44bb572f39dee66c35cbb87 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Thu, 15 Feb 2024 00:37:18 -0600 Subject: [PATCH 13/23] Ignore type, for mypy --- rsconnect/utils_package.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rsconnect/utils_package.py b/rsconnect/utils_package.py index 5d639a02..9ce5fbf8 100644 --- a/rsconnect/utils_package.py +++ b/rsconnect/utils_package.py @@ -156,7 +156,7 @@ def _parse_version_spec(version_spec: str) -> VersionInfo | None: """ for op in (">=", "<=", ">", "<", "==", "!="): if version_spec.startswith(op): - return VersionInfo(op, version_spec[len(op) :].strip()) + return VersionInfo(op, version_spec[len(op) :].strip()) # type: ignore return None From c9de55319c87c75c3c643f0361e2ee025961d5cd Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Thu, 15 Feb 2024 00:42:17 -0600 Subject: [PATCH 14/23] Typing fixes --- rsconnect/api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rsconnect/api.py b/rsconnect/api.py index 3642471a..a8795be0 100644 --- a/rsconnect/api.py +++ b/rsconnect/api.py @@ -8,7 +8,7 @@ import os from os.path import abspath, dirname import time -from typing import IO, Callable, Optional +from typing import BinaryIO, Callable, Optional import base64 import datetime import hashlib @@ -720,7 +720,7 @@ def deploy_bundle( deployment_name: Optional[str] = None, title: Optional[str] = None, title_is_default: bool = False, - bundle: Optional[IO[str] | IO[bytes]] = None, + bundle: Optional[BinaryIO] = None, env_vars: Optional[dict[str, str]] = None, app_mode: Optional[AppMode] = None, visibility: Optional[str] = None, @@ -729,7 +729,7 @@ def deploy_bundle( deployment_name = deployment_name or self.get("deployment_name") title = title or self.get("title") title_is_default = title_is_default or self.get("title_is_default") - bundle = bundle or self.get("bundle") + bundle = bundle or typing.cast(BinaryIO, self.get("bundle")) env_vars = env_vars or self.get("env_vars") app_mode = app_mode or self.get("app_mode") visibility = visibility or self.get("visibility") From 95b73b9d7729aaebf73498f7f2781b1675f077a7 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Thu, 15 Feb 2024 00:45:33 -0600 Subject: [PATCH 15/23] Use typing_extensions on Python 3.7 --- rsconnect/utils_package.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/rsconnect/utils_package.py b/rsconnect/utils_package.py index 9ce5fbf8..e490bb22 100644 --- a/rsconnect/utils_package.py +++ b/rsconnect/utils_package.py @@ -5,7 +5,14 @@ from __future__ import annotations import re -from typing import Literal, NamedTuple, cast +import sys +from typing import NamedTuple, cast + +# This conditional code can be removed when we drop support for Python 3.7. +if sys.version_info >= (3, 8): + from typing import Literal +else: + from typing_extensions import Literal import semver From 69087e6cbc15d4ec2b207f30984cbdeecf32f276 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Thu, 15 Feb 2024 01:10:44 -0600 Subject: [PATCH 16/23] Update docstring --- rsconnect/utils_package.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rsconnect/utils_package.py b/rsconnect/utils_package.py index e490bb22..32cd885a 100644 --- a/rsconnect/utils_package.py +++ b/rsconnect/utils_package.py @@ -160,6 +160,10 @@ def _parse_version_spec(version_spec: str) -> VersionInfo | None: """ Parse a version spec string like ">=1.0" into a tuple like (">=", "1.0"). If it is unable to parse the string, return None. + + For the purposes for which this function is used, it makes sense to return None if + the spec can't be parsed, instead of raising an exception. For our use cases, we do + not want to interrupt the flow of the program if we encounter malformed input. """ for op in (">=", "<=", ">", "<", "==", "!="): if version_spec.startswith(op): From f09f514fbd16528fef0ecaa7cc22221d6f49bd68 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Thu, 15 Feb 2024 01:37:14 -0600 Subject: [PATCH 17/23] Add pyright to pyproject.toml --- pyproject.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 070d5d68..119c05b7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -87,3 +87,6 @@ write_to = "rsconnect/version.py" [tool.pytest.ini_options] markers = ["vetiver: tests for vetiver"] + +[tool.pyright] +typeCheckingMode = "strict" From e6b25fe4caef41558a9eab50499641a0428cb0b2 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Thu, 15 Feb 2024 16:09:00 -0600 Subject: [PATCH 18/23] Remove print statement --- rsconnect/utils_package.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/rsconnect/utils_package.py b/rsconnect/utils_package.py index 32cd885a..8023c9f1 100644 --- a/rsconnect/utils_package.py +++ b/rsconnect/utils_package.py @@ -224,8 +224,6 @@ def fix_starlette_requirements( if not (app_mode == AppModes.PYTHON_SHINY and compare_semvers(connect_version_string, "2024.01.0") == -1): return environment - print(f"Connect server version: {connect_version_string}") - requirements_txt = cast(str, environment.contents) reqs = parse_requirements_txt(requirements_txt) From d0c9a55b054de2d2d6eaf9621a6c0353040cec46 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Thu, 15 Feb 2024 16:09:13 -0600 Subject: [PATCH 19/23] Suppress pyright warninig --- tests/test_utils_package.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_utils_package.py b/tests/test_utils_package.py index 805ae70b..4401ba14 100644 --- a/tests/test_utils_package.py +++ b/tests/test_utils_package.py @@ -1,6 +1,6 @@ from textwrap import dedent from rsconnect.utils_package import ( - _remove_leading_zeros, + _remove_leading_zeros, # pyright: ignore[reportPrivateUsage] compare_package_versions, compare_semvers, parse_requirements_txt, From 216d926f5fb4a867eaa759591fe43dd115f0cc3a Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Fri, 16 Feb 2024 11:42:04 -0600 Subject: [PATCH 20/23] Use logger.warn() instead of print() --- rsconnect/utils_package.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rsconnect/utils_package.py b/rsconnect/utils_package.py index 8023c9f1..083aa82f 100644 --- a/rsconnect/utils_package.py +++ b/rsconnect/utils_package.py @@ -17,6 +17,7 @@ import semver from .environment import Environment +from .log import logger from .models import AppMode, AppModes ComparisonOperator = Literal[">=", "<=", ">", "<", "==", "!="] @@ -250,7 +251,7 @@ def fix_starlette_requirements( if compare_semvers(starlette_req.specs[0].version, "0.35.0") >= 0: # starlette is in requirements.txt, but with a version spec that is # not compatible with this version of Connect. - print( + logger.warn( "Starlette version is 0.35.0 or greater, but this version of Connect " "requires starlette<0.35.0. Setting to <0.35.0." ) From 004c670ce662d9b90f875eab424135bf8ddb017a Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Fri, 16 Feb 2024 11:42:22 -0600 Subject: [PATCH 21/23] Use more specific typing for extra_files --- rsconnect/main.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rsconnect/main.py b/rsconnect/main.py index b9b5b143..812383a6 100644 --- a/rsconnect/main.py +++ b/rsconnect/main.py @@ -1405,7 +1405,7 @@ def deploy_app( force_generate: bool, verbose: int, directory: str, - extra_files: tuple[str, ...] | list[str], + extra_files: tuple[str, ...], visibility: Optional[str], env_vars: dict[str, str], image: str, @@ -1419,7 +1419,7 @@ def deploy_app( ): set_verbosity(verbose) entrypoint = validate_entry_point(entrypoint, directory) - extra_files = validate_extra_files(directory, extra_files) + extra_files_list = validate_extra_files(directory, extra_files) environment = create_python_environment( directory, force_generate, @@ -1469,7 +1469,7 @@ def deploy_app( entrypoint, app_mode, environment, - extra_files, + extra_files_list, exclude, image=image, env_management_py=env_management_py, From 0a2fef1a0eb6be37e7a190f9779bdb2136cd6ebb Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Fri, 16 Feb 2024 11:44:49 -0600 Subject: [PATCH 22/23] Add pathlib.Path type --- rsconnect/bundle.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rsconnect/bundle.py b/rsconnect/bundle.py index 2cc9c587..d875ddb2 100644 --- a/rsconnect/bundle.py +++ b/rsconnect/bundle.py @@ -817,7 +817,7 @@ def create_glob_set(directory, excludes): return GlobSet(work) -def is_environment_dir(directory): +def is_environment_dir(directory: str | Path): """Detect whether `directory` is a virtualenv""" # A virtualenv will have Python at ./bin/python @@ -1506,7 +1506,7 @@ def _warn_if_no_requirements_file(directory: str): ) -def _warn_if_environment_directory(directory: str): +def _warn_if_environment_directory(directory: str | Path): """ Issue a warning if the deployment directory is itself a virtualenv (yikes!). From dbd10b1a76e5f5a499a09084e81566e1f98da172 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Mon, 26 Feb 2024 14:24:07 -0600 Subject: [PATCH 23/23] Only try to get Connect version when using Connect server --- rsconnect/main.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/rsconnect/main.py b/rsconnect/main.py index 812383a6..e0be28a2 100644 --- a/rsconnect/main.py +++ b/rsconnect/main.py @@ -1453,13 +1453,14 @@ def deploy_app( **extra_args, # type: ignore ) - # Update the starlette version if needed. After all users are on Connect - # 2024.01.1 or later, this can be removed. - environment = fix_starlette_requirements( - environment=environment, - app_mode=app_mode, - connect_version_string=cast(str, ce.server_details["connect"]), # type: ignore - ) + if isinstance(ce.client, RSConnectClient): + # Update the starlette version if needed. After all users are on Connect + # 2024.01.1 or later, this can be removed. + environment = fix_starlette_requirements( + environment=environment, + app_mode=app_mode, + connect_version_string=cast(str, ce.client.server_settings()["version"]), + ) ce.validate_server() ce.validate_app_mode(app_mode=app_mode)