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

Consider vendoring python-mimeparse #1420

Closed
vytas7 opened this issue Jan 26, 2019 · 3 comments
Closed

Consider vendoring python-mimeparse #1420

vytas7 opened this issue Jan 26, 2019 · 3 comments

Comments

@vytas7
Copy link
Member

vytas7 commented Jan 26, 2019

As brought up on #1383 and discussions on Gitter, we may consider vendoring python-mimeparse dependency.

Normally, vendoring Python dependencies is just an unnecessary burden:

  • Laborious process to keep integrating upstream fixes
  • Pip is good at handling dependencies
  • Redundant duplication of libraries if the application is also using them

However, in this particular case of python-mimeparse it might be a compelling choice:

  • As we are no longer relying on six, this would make Falcon completely dependency-free, which, while not being much of a benefit in the virtualenv + Pip workflow, might ease distributing Falcon in other ways, such as packaging for Linux distributions (see for instance: mimeparse vs python-mimeparse python-mimeparse#20 ⬅️ note by @kgriffs )
  • python-mimeparse is a mature package with a concise codebase (cone 190-line file), the pace of recent developments is rather moderate (at the time of writing, Jan 2019, the latest stable release on PyPi is from 2016)
  • We would cythonize it together with the rest of Falcon, see also Contribute optimizations to the python-mimeparse project #1183

If we decide to do it, we need to keep an eye on upstream developments. We could introduce a Tox or Travis check that we have not fallen behind its stable PyPi version.

@kgriffs kgriffs added this to the Version 2.0 milestone Jan 28, 2019
kgriffs added a commit to kgriffs/falcon that referenced this issue Feb 12, 2019
kgriffs added a commit to kgriffs/falcon that referenced this issue Feb 12, 2019
kgriffs added a commit to kgriffs/falcon that referenced this issue Feb 12, 2019
kgriffs added a commit to kgriffs/falcon that referenced this issue Feb 12, 2019
kgriffs added a commit to kgriffs/falcon that referenced this issue Feb 12, 2019
kgriffs added a commit to kgriffs/falcon that referenced this issue Feb 12, 2019
kgriffs added a commit to kgriffs/falcon that referenced this issue Feb 12, 2019
kgriffs added a commit to kgriffs/falcon that referenced this issue Feb 12, 2019
kgriffs added a commit to kgriffs/falcon that referenced this issue Feb 12, 2019
jmvrbanac pushed a commit that referenced this issue Feb 12, 2019
@Apteryks
Copy link

I'd rather have the option to use the regular mimeparse dependency, for packaging purposes (GNU Guix). As it stands, we unbundled it already but it recently started failing like so:


    def test_deps_mimeparse_correct_package():
        """Ensure we are dealing with python-mimeparse, not mimeparse."""
    
>       tokens = mimeparse.mimeparse.__version__.split('.')
E       AttributeError: module 'mimeparse' has no attribute 'mimeparse'

tests/test_deps.py:11: AttributeError

Probably because of changes in mimeparse upstream.

@vytas7
Copy link
Member Author

vytas7 commented Apr 18, 2022

Hi @Apteryks!
We're sorry this has caused you problems.

FWIW, python-mimeparse hasn't seen any release in a long while, so I doubt any upstream changes could be an issue per se. We have now taken over control of python-mimeparse and moved its maintenance under the Falconry umbrella, so we'll try to refine a way forward, potentially including an option to unbundle.

And I can indeed reproduce the issue, probably we haven't really payed attention to the possibility of unbundling (although I know Fedora Linux (and possibly Debian too) does unbundle, but I haven't looked how exactly they do that). NB that you will need a patch to replace the vendored imports with the global ones in either case.

I'll file an issue so that make a decision going forward.

@Apteryks
Copy link

Apteryks commented Apr 18, 2022

Hello @vytas7 and thank you for the prompt reply. You are correct that mimeparse hasn't changed upstream in recent years, it seems. The problem would have rather been introduced with f19039a; which accesses the __version__ variable through the mimeparse.mimeparse namespace rather than just mimeparse. Reverting to the previous mimeparse.__version__ works for me (we patch the top level import to read as just import mimeparse and provide the extra unbundled dependency).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants