-
Notifications
You must be signed in to change notification settings - Fork 37
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
Move ShellLauncher
#658
Move ShellLauncher
#658
Conversation
…abs#653) This PR mitigates two issues encountered during installation on build agents ## mypy/typing_extensions Installation of mypy or dragon in separate build actions caused some dependencies (typing_extensions, numpy) to be upgraded. Those upgrades result in runtime failures. The build actions were tweaked to allow pip to consider all optional dependencies during resolution. ## dragon/numpy Additionally, the numpy version was capped on dragon installations. [ committed by @ankona] [ approved by @ashao @MattToast ]
The builder module was included in `setup.py` to allow us to ship the main Redis binaries (not RedisAI) with installs from PyPI. The changes in this PR remove our ability to do this and requires users to build Redis as part of the `smart build`. This change in behaviour was deemed reasonable to allow for easier maintenance and extension of the Builder class as well as simplify the deployment of wheels. [ committed by @ashao ] [ reviewed by @MattToast ]
The version of codecov has been updated to v4.5.0 for the github actions. [ committed by @mellis13 ] [ reviewed by @amandarichardsonn ]
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.
One quick question before approving
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.
A couple of small nits and a couple of philosophy questions on where we think some of the protocols should "live", but otherwise looks about ready to go on my end!
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.
Looks great! I have one small philosophy question, but nothing worth holding up approval over.
LGTM!! Thanks for helping to keep us organized!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## smartsim-refactor #658 +/- ##
====================================================
Coverage ? 40.45%
====================================================
Files ? 110
Lines ? 7326
Branches ? 0
====================================================
Hits ? 2964
Misses ? 4362
Partials ? 0
|
0569c05
into
CrayLabs:smartsim-refactor
No description provided.