Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REF-2676][REF-2751]Windows Skip ARM devices on bun install + Telemetry #3212

Merged
merged 9 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions reflex/constants/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
COOKIES,
ENV_MODE_ENV_VAR,
IS_WINDOWS,
IS_WINDOWS_BUN_SUPPORTED_MACHINE, # type: ignore
LOCAL_STORAGE,
POLLING_MAX_HTTP_BUFFER_SIZE,
PYTEST_CURRENT_TEST,
Expand Down Expand Up @@ -87,7 +86,6 @@
Hooks,
Imports,
IS_WINDOWS,
IS_WINDOWS_BUN_SUPPORTED_MACHINE,
LOCAL_STORAGE,
LogLevel,
MemoizationDisposition,
Expand Down
5 changes: 0 additions & 5 deletions reflex/constants/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@
from platformdirs import PlatformDirs

IS_WINDOWS = platform.system() == "Windows"
# https://github.com/oven-sh/bun/blob/main/src/cli/install.ps1
IS_WINDOWS_BUN_SUPPORTED_MACHINE = IS_WINDOWS and platform.machine() in [
"AMD64",
"x86_64",
] # filter out 32 bit + ARM


class Dirs(SimpleNamespace):
Expand Down
6 changes: 5 additions & 1 deletion reflex/utils/exec.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,11 @@ def output_system_info():

system = platform.system()

if system != "Windows":
if (
system != "Windows"
or system == "Windows"
and prerequisites.is_windows_bun_supported()
):
dependencies.extend(
[
f"[FNM {prerequisites.get_fnm_version()} (Expected: {constants.Fnm.VERSION}) (PATH: {constants.Fnm.EXE})]",
Expand Down
104 changes: 101 additions & 3 deletions reflex/utils/prerequisites.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from __future__ import annotations

import functools
import glob
import importlib
import inspect
Expand Down Expand Up @@ -49,6 +50,14 @@ class Template(Base):
demo_url: str


class CpuInfo(Base):
"""Model to save cpu info."""

manufacturer_id: Optional[str]
model_name: Optional[str]
address_width: Optional[int]


def check_latest_package_version(package_name: str):
"""Check if the latest version of the package is installed.

Expand Down Expand Up @@ -172,7 +181,7 @@ def get_install_package_manager() -> str | None:
Returns:
The path to the package manager.
"""
if constants.IS_WINDOWS and not constants.IS_WINDOWS_BUN_SUPPORTED_MACHINE:
if constants.IS_WINDOWS and not is_windows_bun_supported():
return get_package_manager()
return get_config().bun_path

Expand Down Expand Up @@ -728,7 +737,7 @@ def install_bun():
Raises:
FileNotFoundError: If required packages are not found.
"""
if constants.IS_WINDOWS and not constants.IS_WINDOWS_BUN_SUPPORTED_MACHINE:
if constants.IS_WINDOWS and not is_windows_bun_supported():
console.warn(
"Bun for Windows is currently only available for x86 64-bit Windows. Installation will fall back on npm."
)
Expand Down Expand Up @@ -826,7 +835,7 @@ def install_frontend_packages(packages: set[str], config: Config):
get_package_manager()
if not constants.IS_WINDOWS
or constants.IS_WINDOWS
and constants.IS_WINDOWS_BUN_SUPPORTED_MACHINE
and is_windows_bun_supported()
else None
)
processes.run_process_with_fallback(
Expand Down Expand Up @@ -1411,3 +1420,92 @@ def initialize_app(app_name: str, template: str | None = None):
)

telemetry.send("init", template=template)


def format_address_width(address_width) -> int | None:
"""Cast address width to an int.

Args:
address_width: The address width.

Returns:
Address width int
"""
try:
return int(address_width) if address_width else None
except ValueError:
return None


@functools.lru_cache(maxsize=None)
def get_cpu_info() -> CpuInfo | None:
"""Get the CPU info of the underlining host.

Returns:
The CPU info.
"""
platform_os = platform.system()
cpuinfo = {}
try:
if platform_os == "Windows":
cmd = "wmic cpu get addresswidth,caption,manufacturer /FORMAT:csv"
output = processes.execute_command_and_return_output(cmd)
if output:
val = output.splitlines()[-1].split(",")[1:]
cpuinfo["manufacturer_id"] = val[2]
cpuinfo["model_name"] = val[1].split("Family")[0].strip()
cpuinfo["address_width"] = format_address_width(val[0])
elif platform_os == "Linux":
output = processes.execute_command_and_return_output("lscpu")
if output:
lines = output.split("\n")
for line in lines:
if "Architecture" in line:
cpuinfo["address_width"] = (
64 if line.split(":")[1].strip() == "x86_64" else 32
)
if "Vendor ID:" in line:
cpuinfo["manufacturer_id"] = line.split(":")[1].strip()
if "Model name" in line:
cpuinfo["model_name"] = line.split(":")[1].strip()
elif platform_os == "Darwin":
cpuinfo["address_width"] = format_address_width(
processes.execute_command_and_return_output("getconf LONG_BIT")
)
cpuinfo["manufacturer_id"] = processes.execute_command_and_return_output(
"sysctl -n machdep.cpu.brand_string"
)
cpuinfo["model_name"] = processes.execute_command_and_return_output(
"uname -m"
)
except Exception as err:
console.error(f"Failed to retrieve CPU info. {err}")
return None

return (
CpuInfo(
manufacturer_id=cpuinfo.get("manufacturer_id"),
model_name=cpuinfo.get("model_name"),
address_width=cpuinfo.get("address_width"),
)
if cpuinfo
else None
)


@functools.lru_cache(maxsize=None)
def is_windows_bun_supported() -> bool:
"""Check whether the underlining host running windows qualifies to run bun.
We typically do not run bun on ARM or 32 bit devices that use windows.

Returns:
Whether the host is qualified to use bun.
"""
cpu_info = get_cpu_info()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we should wrap this in a try/except; reflex shouldn't fail to work just because a user's system doesn't have lscpu or the address width isn't castable as an int for whatever reason.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, pushed a fix for this

return (
constants.IS_WINDOWS
and cpu_info is not None
and cpu_info.address_width == 64
and cpu_info.model_name is not None
and "ARM" not in cpu_info.model_name
)
18 changes: 18 additions & 0 deletions reflex/utils/processes.py
Original file line number Diff line number Diff line change
Expand Up @@ -347,3 +347,21 @@ def run_process_with_fallback(args, *, show_status_message, fallback=None, **kwa
fallback=None,
**kwargs,
)


def execute_command_and_return_output(command) -> str | None:
"""Execute a command and return the output.

Args:
command: The command to run.

Returns:
The output of the command.
"""
try:
return subprocess.check_output(command, shell=True).decode().strip()
except subprocess.SubprocessError as err:
console.error(
f"The command `{command}` failed with error: {err}. This will return None."
)
return None
17 changes: 16 additions & 1 deletion reflex/utils/telemetry.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ def get_os() -> str:
return platform.system()


def get_detailed_platform_str() -> str:
"""Get the detailed os/platform string.

Returns:
The platform string
"""
return platform.platform()


def get_python_version() -> str:
"""Get the Python version.

Expand Down Expand Up @@ -97,6 +106,8 @@ def _prepare_event(event: str, **kwargs) -> dict:
Returns:
The event data.
"""
from reflex.utils.prerequisites import get_cpu_info

installation_id = ensure_reflex_installation_id()
project_hash = get_project_hash(raise_on_fail=_raise_on_missing_project_hash())

Expand All @@ -112,17 +123,22 @@ def _prepare_event(event: str, **kwargs) -> dict:
else:
# for python 3.11 & 3.12
stamp = datetime.now(UTC).isoformat()

cpuinfo = get_cpu_info()

return {
"api_key": "phc_JoMo0fOyi0GQAooY3UyO9k0hebGkMyFJrrCw1Gt5SGb",
"event": event,
"properties": {
"distinct_id": installation_id,
"distinct_app_id": project_hash,
"user_os": get_os(),
"user_os_detail": get_detailed_platform_str(),
"reflex_version": get_reflex_version(),
"python_version": get_python_version(),
"cpu_count": get_cpu_count(),
"memory": get_memory(),
"cpu_info": dict(cpuinfo) if cpuinfo else {},
**(
{"template": template}
if (template := kwargs.get("template")) is not None
Expand Down Expand Up @@ -165,5 +181,4 @@ def send(event: str, telemetry_enabled: bool | None = None, **kwargs) -> bool:
event_data = _prepare_event(event, **kwargs)
if not event_data:
return False

return _send_event(event_data)
13 changes: 13 additions & 0 deletions tests/test_prerequisites.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
from reflex import constants
from reflex.config import Config
from reflex.utils.prerequisites import (
CpuInfo,
_update_next_config,
cached_procedure,
get_cpu_info,
initialize_requirements_txt,
)

Expand Down Expand Up @@ -203,3 +205,14 @@ def _function_with_some_args(*args, **kwargs):
assert call_count == 2
_function_with_some_args(100, y=300)
assert call_count == 2


def test_get_cpu_info():
cpu_info = get_cpu_info()
assert cpu_info is not None
assert isinstance(cpu_info, CpuInfo)
assert cpu_info.model_name is not None

for attr in ("manufacturer_id", "model_name", "address_width"):
value = getattr(cpu_info, attr)
assert value.strip() if attr != "address_width" else value
1 change: 1 addition & 0 deletions tests/test_telemetry.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def test_send(mocker, event):
read_data='{"project_hash": "78285505863498957834586115958872998605"}'
),
)
mocker.patch("platform.platform", return_value="Mocked Platform")

telemetry.send(event, telemetry_enabled=True)
httpx.post.assert_called_once()
Expand Down
Loading