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

Conversation

ElijahAhianyo
Copy link
Collaborator

@ElijahAhianyo ElijahAhianyo commented May 2, 2024

Previously, we used python.machine() to get the cpu architecture of the underlining host, but since the architecture produced by python is only that of the executable python binary installed on the host, it may not correlate with the actual architecture of the host. A classic eg. Is Installing the x86_64 version of python on an ARM device. This causes reflex to install bun which later fails when installing frontend packages and then fall back to npm.
In this PR, we use wmic on Windows to get accurate info on the host CPU architecture, which should skip bun installation completely on ARM and 32-bit devices.

In addition to this PR, we're collecting more telemetry data on the OS(previously we only collected the OS name without the version and build) and CPU; model, manufacturer or vendor ID and address width (whether the underlining host is 32 or 64 bit). This should aid us and give us a fair idea in the future where/when necessary during development, the nature of devices users run.

fixes #3151

@ElijahAhianyo ElijahAhianyo changed the title [WIP]CPU Info and Telemetry Windows Skip ARM devices on bun install + Telemetry May 2, 2024
@ElijahAhianyo ElijahAhianyo changed the title Windows Skip ARM devices on bun install + Telemetry [REF-2676][REF-2751]Windows Skip ARM devices on bun install + Telemetry May 2, 2024
@ElijahAhianyo ElijahAhianyo marked this pull request as ready for review May 2, 2024 15:14
masenf
masenf previously approved these changes May 2, 2024
Copy link
Collaborator

@masenf masenf left a comment

Choose a reason for hiding this comment

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

Working for me on ARM and Azure x86_64

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

reflex/utils/telemetry.py Outdated Show resolved Hide resolved
@picklelo picklelo merged commit 0838e5a into main May 6, 2024
46 checks passed
@picklelo picklelo deleted the elijah/get-architecture branch August 21, 2024 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[REF-2676] Bun should not be used for ARM64 Windows
3 participants