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

Have build script fail on non-successful build of native (cython) extension and add explicit flag to use fallback #481

Closed
frittentheke opened this issue Jul 28, 2021 · 1 comment

Comments

@frittentheke
Copy link

frittentheke commented Jul 28, 2021

I recently debugged a tricky issue with OpenStack Neutron or rather oslo.privsep (https://github.com/openstack/oslo.privsep) which makes heavy use of your very appreciated python-msgpack. You'll find the whole story here: https://bugs.launchpad.net/cloud-archive/+bug/1937261.

The root cause for my observed issue was that the python-msgpack backport of version 0.6.2 to Ubuntu Bionic done by Ubuntu Cloud Archive was using the pure python fallback for pack/unpack. And this was simply due to Cython being too old as it lacked the support for bytearray which was only added in 0.29 cython/cython#2573.

  1. The version requirement stated in the warning at

    print("Install Cython >= 0.16 or install msgpack from PyPI.")
    is not correct anymore, obviously >=0.29 is true at least for python-msgpack version 0.6.2 and newer.

  2. Honestly I believe your build script is just way too nice, skipping over every possible error condition when building the extension and then simply falling back to pure python:

    class BuildExt(build_ext):

    I'd like to suggest to fail hard if Cython is not available or cannot build successfully and then add a flag to not build the extension and explicitly use the pure python fallback.
    In this particular case only looking at the human readable build logs exposed this problem which only gets harder to debug with every additional layer.

  3. Not allowing an at-runtime fallback is a whole other story - but could be sensible as well for use cases which will just not work with the performance available with fallback.

@methane
Copy link
Member

methane commented Jul 29, 2021

Note that it is not relating to most users.

  • binary wheels are provided for common platforms.
  • Generated source file is bundled in source package (.tar.gz) so user do not need to run Cython.

@methane methane closed this as completed Nov 16, 2021
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

No branches or pull requests

2 participants