-
-
Notifications
You must be signed in to change notification settings - Fork 228
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
Add proper condition for installing uvloop #595
Conversation
✅ Deploy Preview for robyn canceled.
|
2ab58b1
to
49c1872
Compare
@banipreetr done 😄 |
CodSpeed Performance ReportMerging #595 will not alter performanceComparing Summary
|
pyproject.toml
Outdated
@@ -30,7 +30,7 @@ dependencies = [ | |||
'nestd == 0.3.1', | |||
'inquirerpy == 0.3.4', | |||
# conditional | |||
"uvloop>=0.17.0; sys_platform == 'darwin' and platform_machine == 'aarch_64' and platform_machine == 'x86_64' and platform_machine == 'i686'" | |||
"uvloop>=0.17.0; sys_platform == 'darwin' and (platform_machine == 'aarch_64' or platform_machine == 'x86_64' or platform_machine == 'i686')" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am pretty sure uvloop
is not available on all aarch_64
platforms.
This will still not work imo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition will only install uvloop on osx and not other platforms.
pyproject.toml
Outdated
@@ -30,7 +30,7 @@ dependencies = [ | |||
'nestd == 0.3.1', | |||
'inquirerpy == 0.3.4', | |||
# conditional | |||
"uvloop>=0.17.0; sys_platform == 'darwin' and platform_machine == 'aarch_64' and platform_machine == 'x86_64' and platform_machine == 'i686'" | |||
"uvloop~=0.17.0; sys_platform != 'win32' and platform_python_implementation == 'CPython'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @sansyrox ! Thanks for pushing this. Can you please explain the condition:
platform_python_implementation == 'CPython'
Why is it needed here in Robyn's case?
Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @banipreetr 👋
This is needed when the users try to use robyn with PyPy
. While we don't explicitly show support for PyPy on our repo. I am 70% sure that robyn works with PyPy lol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🔥
Description
This PR fixes #594