From 9da65b9a9a1ee177bb749de5e95e1f2272a14df9 Mon Sep 17 00:00:00 2001 From: Martin Xu <15661672+martinxu9@users.noreply.github.com> Date: Thu, 14 Dec 2023 14:25:40 -0800 Subject: [PATCH] [REF-1464] Handle requirements.txt encoding (#2284) --- poetry.lock | 4 +-- pyproject.toml | 1 + reflex/constants/config.py | 2 +- reflex/utils/prerequisites.py | 24 ++++++++++++--- tests/test_prerequisites.py | 57 ++++++++++++++++++++++++++++++++--- 5 files changed, 75 insertions(+), 13 deletions(-) diff --git a/poetry.lock b/poetry.lock index 44cd29146..b94d138ed 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.6.1 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.7.1 and should not be changed by hand. [[package]] name = "alembic" @@ -2473,4 +2473,4 @@ testing = ["big-O", "jaraco.functools", "jaraco.itertools", "more-itertools", "p [metadata] lock-version = "2.0" python-versions = "^3.8" -content-hash = "3b45d79329803bc24d8874a6da7af8f6eb6c4677eb100cbe300ad6f9e4bc8c69" +content-hash = "c22317cf6beac82268e73619e984788895d258bb7b7983eacdfbf6093c419dc3" diff --git a/pyproject.toml b/pyproject.toml index cbce05edd..00dc50a49 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -58,6 +58,7 @@ wrapt = [ packaging = "^23.1" pipdeptree = "^2.13.0" reflex-hosting-cli = ">=0.1.2" +charset-normalizer = "^3.3.2" [tool.poetry.group.dev.dependencies] pytest = "^7.1.2" diff --git a/reflex/constants/config.py b/reflex/constants/config.py index df56ecff3..298934202 100644 --- a/reflex/constants/config.py +++ b/reflex/constants/config.py @@ -47,7 +47,7 @@ class RequirementsTxt(SimpleNamespace): # The requirements.txt file. FILE = "requirements.txt" # The partial text used to form requirement that pins a reflex version - DEFAULTS_STUB = f"{Reflex.MODULE_NAME}>=" + DEFAULTS_STUB = f"{Reflex.MODULE_NAME}==" # The deployment URL. diff --git a/reflex/utils/prerequisites.py b/reflex/utils/prerequisites.py index 9d542e9a6..2b9c6d8c3 100644 --- a/reflex/utils/prerequisites.py +++ b/reflex/utils/prerequisites.py @@ -159,7 +159,6 @@ def get_app(reload: bool = False) -> ModuleType: sys.path.insert(0, os.getcwd()) app = __import__(module, fromlist=(constants.CompileVars.APP,)) if reload: - importlib.reload(app) return app @@ -263,18 +262,33 @@ def initialize_requirements_txt(): the requirements.txt file. """ fp = Path(constants.RequirementsTxt.FILE) - fp.touch(exist_ok=True) + encoding = "utf-8" + if not fp.exists(): + fp.touch() + else: + # Detect the encoding of the original file + import charset_normalizer + charset_matches = charset_normalizer.from_path(fp) + maybe_charset_match = charset_matches.best() + if maybe_charset_match is None: + console.debug(f"Unable to detect encoding for {fp}, exiting.") + return + encoding = maybe_charset_match.encoding + console.debug(f"Detected encoding for {fp} as {encoding}.") try: - with open(fp, "r") as f: + other_requirements_exist = False + with open(fp, "r", encoding=encoding) as f: for req in f.readlines(): # Check if we have a package name that is reflex if re.match(r"^reflex[^a-zA-Z0-9]", req): console.debug(f"{fp} already has reflex as dependency.") return - with open(fp, "a") as f: + other_requirements_exist = True + with open(fp, "a", encoding=encoding) as f: + preceding_newline = "\n" if other_requirements_exist else "" f.write( - f"\n{constants.RequirementsTxt.DEFAULTS_STUB}{constants.Reflex.VERSION}\n" + f"{preceding_newline}{constants.RequirementsTxt.DEFAULTS_STUB}{constants.Reflex.VERSION}\n" ) except Exception: console.info(f"Unable to check {fp} for reflex dependency.") diff --git a/tests/test_prerequisites.py b/tests/test_prerequisites.py index ff2c44661..846c9d7eb 100644 --- a/tests/test_prerequisites.py +++ b/tests/test_prerequisites.py @@ -1,4 +1,4 @@ -from unittest.mock import mock_open +from unittest.mock import Mock, mock_open import pytest @@ -56,24 +56,37 @@ def test_update_next_config(config, export, expected_output): assert output == expected_output -def test_initialize_requirements_txt(mocker): +def test_initialize_requirements_txt_no_op(mocker): # File exists, reflex is included, do nothing - mocker.patch("os.path.exists", return_value=True) + mocker.patch("pathlib.Path.exists", return_value=True) + mocker.patch( + "charset_normalizer.from_path", + return_value=Mock(best=lambda: Mock(encoding="utf-8")), + ) + mock_fp_touch = mocker.patch("pathlib.Path.touch") open_mock = mock_open(read_data="reflex==0.2.9") mocker.patch("builtins.open", open_mock) initialize_requirements_txt() assert open_mock.call_count == 1 + assert open_mock.call_args.kwargs["encoding"] == "utf-8" assert open_mock().write.call_count == 0 + mock_fp_touch.assert_not_called() def test_initialize_requirements_txt_missing_reflex(mocker): # File exists, reflex is not included, add reflex + mocker.patch("pathlib.Path.exists", return_value=True) + mocker.patch( + "charset_normalizer.from_path", + return_value=Mock(best=lambda: Mock(encoding="utf-8")), + ) open_mock = mock_open(read_data="random-package=1.2.3") mocker.patch("builtins.open", open_mock) initialize_requirements_txt() # Currently open for read, then open for append assert open_mock.call_count == 2 - assert open_mock().write.call_count == 1 + for call_args in open_mock.call_args_list: + assert call_args.kwargs["encoding"] == "utf-8" assert ( open_mock().write.call_args[0][0] == f"\n{constants.RequirementsTxt.DEFAULTS_STUB}{constants.Reflex.VERSION}\n" @@ -82,12 +95,46 @@ def test_initialize_requirements_txt_missing_reflex(mocker): def test_initialize_requirements_txt_not_exist(mocker): # File does not exist, create file with reflex - mocker.patch("os.path.exists", return_value=False) + mocker.patch("pathlib.Path.exists", return_value=False) open_mock = mock_open() mocker.patch("builtins.open", open_mock) initialize_requirements_txt() assert open_mock.call_count == 2 + # By default, use utf-8 encoding + for call_args in open_mock.call_args_list: + assert call_args.kwargs["encoding"] == "utf-8" assert open_mock().write.call_count == 1 + assert ( + open_mock().write.call_args[0][0] + == f"{constants.RequirementsTxt.DEFAULTS_STUB}{constants.Reflex.VERSION}\n" + ) + + +def test_requirements_txt_cannot_detect_encoding(mocker): + mocker.patch("pathlib.Path.exists", return_value=True) + mock_open = mocker.patch("builtins.open") + mocker.patch( + "charset_normalizer.from_path", + return_value=Mock(best=lambda: None), + ) + initialize_requirements_txt() + mock_open.assert_not_called() + + +def test_requirements_txt_other_encoding(mocker): + mocker.patch("pathlib.Path.exists", return_value=True) + mocker.patch( + "charset_normalizer.from_path", + return_value=Mock(best=lambda: Mock(encoding="utf-16")), + ) + initialize_requirements_txt() + open_mock = mock_open(read_data="random-package=1.2.3") + mocker.patch("builtins.open", open_mock) + initialize_requirements_txt() + # Currently open for read, then open for append + assert open_mock.call_count == 2 + for call_args in open_mock.call_args_list: + assert call_args.kwargs["encoding"] == "utf-16" assert ( open_mock().write.call_args[0][0] == f"\n{constants.RequirementsTxt.DEFAULTS_STUB}{constants.Reflex.VERSION}\n"