-
Notifications
You must be signed in to change notification settings - Fork 137
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
chore: Bundle all Ruby standalone binaries into wheel when deploying #256
Conversation
Thanks @fresto32 ! I think this PR is helpful, but I think a more helpful one would be to package the standalone into the pip package - is that possible? With the proposed approach, you now need to sync an offline package (the standalone) with the pip package. Ideally, the whole package can be self contained. |
Wow that was a quick turnaround. Thanks - yeah, if you're happy to bundle the current |
I think that would be preferable. Elliot/Matt may have other ideas though? |
Install all ruby standalone packages when running sdist. Devloppers can also now provide a --bin-path that contains the binaries required for their OS, so that can they use this package in offline environments.
We've made some adjustments to
Please let me know if there's anything else we can do |
python setup.py sdist
python setup.py sdist
Awesome, thanks so much! I'll leave it to the maintainers of pact-python to review. |
if download_bin_path is not None: | ||
if os.path.isfile(path) is not True: | ||
raise RuntimeError('Could not find {} binary.'.format(path)) | ||
extract_ruby_app_binary(download_bin_path, package_bin_path, binary['filename']) |
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.
Is this line reachable?
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 think this code will be hit if the user supplies a bin-path
(i.e. download_bin_path
) CLI argument and it contains the necessary standalone binaries
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.
@fresto32 please note that in line 108 we check the opposite condition, so therefore download_bin_path is not None
will never be false. I think the current behavior is broken, when I try to install, the binary is not downloaded currently. It worked for me after I removed the if and left:
download_ruby_app_binary(download_bin_path, binary['filename'], binary['suffix'])
extract_ruby_app_binary(download_bin_path, package_bin_path, binary['filename'])
Sorry for being awol. This is looking pretty good. Thank you for the awesome contribution! Had one minor comment but I'm going to merge anyway. |
Thanks Elliot! 🥳 Feels great to contribute to Pact. |
* chore: Releasing version 1.4.0 * fix: make uvicorn versions over 0.14 (pact-foundation#255) * chore: Releasing version 1.4.1 * chore: Bundle Ruby standalones into dist artifact. (pact-foundation#256) Install all ruby standalone packages when running sdist. Devloppers can also now provide a --bin-path that contains the binaries required for their OS, so that can they use this package in offline environments. * chore: Releasing version 1.4.2 Co-authored-by: Elliott Murray <elliottmurray@gmail.com> Co-authored-by: Taj Pereira <tajpereira@gmail.com>
Install all ruby standalone packages when running sdist. Devloppers can also now provide a --bin-path that contains the binaries required for their OS, so that can they use this package in offline environments.
This pull request modifies
setup.py
to allow for offline installation of this package.In a corporate environment, it is very difficult to garner adoption of a product that is difficult to install in an offline environment. This PR hopes to smooth out that process.
Users can now install this package via
pip install pact-python --bin-path=/../../../
to prevent the installation process from trying to fetch binaries from github.com.This seems to be a short term solution to #116. I am not sure whether the maintainers would want to close that issue or keep it open for the time being. As per @mefellows, "... the ideal state I think is that Python bundles the Ruby application into the package, so at install time no network is required." But this is planned for the FFI interface, which is still in development.
Also, this is my first time working within the pact-foundation, so please excuse any mistakes / issues I unintendedly may have made.