marrow / mailer

A light-weight, modular, message representation and mail delivery framework for Python.
MIT License
276 stars 62 forks source link

Broken in 3.8 due to bad import from marrow/cgi #87

Closed iJebus closed 2 years ago

iJebus commented 4 years ago

Looks like it's broken on python 3.8.1 currently.

Installs fine:

❯ pip install marrow.mailer --user                                                                                                         
Collecting marrow.mailer
  Using cached https://files.pythonhosted.org/packages/8c/37/961103b20bad1a8af86fa67a53748c2bf26e9e242a79fa89dc762d374ea6/marrow.mailer-4.0.3-py2.py3-none-any.whl
Collecting marrow.util<2.0
  Using cached https://files.pythonhosted.org/packages/d2/19/630a0984ba6d5dc4432ed45d2c9ab8752542fa6ba1590a0e72d1fb742c2e/marrow.util-1.2.3.tar.gz
Installing collected packages: marrow.util, marrow.mailer
    Running setup.py install for marrow.util ... done
Successfully installed marrow.mailer-4.0.3 marrow.util-1.2.3

Running a script with the example code:

from marrow.mailer import Mailer, Message

mailer = Mailer(dict(
        transport = dict(
                use = 'smtp',
                host = 'localhost')))
mailer.start()

message = Message(author="user@example.com", to="user-two@example.com")
message.subject = "Testing Marrow Mailer"
message.plain = "This is a test."
mailer.send(message)

mailer.stop()
Traceback (most recent call last):
  File "/home/liam/projects/soc_siem_bi_integration/soc_scripts/cleanup.py", line 8, in <module>
    from marrow.mailer import Mailer, Message
  File "/home/liam/.local/lib/python3.8/site-packages/marrow/mailer/__init__.py", line 12, in <module>
    from marrow.mailer.message import Message
  File "/home/liam/.local/lib/python3.8/site-packages/marrow/mailer/message.py", line 21, in <module>
    from marrow.mailer.address import Address, AddressList, AutoConverter
  File "/home/liam/.local/lib/python3.8/site-packages/marrow/mailer/address.py", line 12, in <module>
    from marrow.util.compat import basestring, unicode, unicodestr, native
  File "/home/liam/.local/lib/python3.8/site-packages/marrow/util/compat.py", line 33, in <module>
    from cgi import parse_qsl
ImportError: cannot import name 'parse_qsl' from 'cgi' (/usr/lib/python3.8/cgi.py)

If I go diving into compat.py and change from cgi import parse_qsl to from urllib.parse import parse_qsl, it works fine. This is makes sense, given Python 3.8 notes:

parse_qs, parse_qsl, and escape are removed from the cgi module. They are deprecated in Python 3.2 or older. They should be imported from the urllib.parse and html modules instead.

I don't know the ramifications of using urllib.parse over html, though.

Util is archived, otherwise I would have raised the issue there /shrug.

nphilipp commented 4 years ago

I don't know the ramifications of using urllib.parse over html, though.

It's just that parse_qs and parse_qsl live in urllib.parse, escape lives in html.

Util is archived, otherwise I would have raised the issue there /shrug.

Yeah, it's kinda odd that marrow.mailer still depends on it.

amcgregor commented 4 years ago

I don't know the ramifications of using urllib.parse over html, though.

Given it's not even used, the solution here is removal of that dependency, which is primarily there to support Python 2 compatibility, which is no longer a desired feature or concern. There are no ramifications.

iamareebjamal commented 4 years ago

Yes, parse_ is not present in this repo, because it is present in marrow.util. Which is indeed still used in quite a few places in the project https://github.com/marrow/mailer/search?q=marrow.util&unscoped_q=marrow.util

amcgregor commented 4 years ago

@iamareebjamal Reference my recent comment on #90 — with a properly quoted search you find the summary included at the end of that comment. That is: virtually none of marrow.util is required, except the compatibility for Python 2, which can go away, and Bunch… which is just an over-engineered attribute access dictionary. Splitting the discussion, though, is quite unfortunate, on this. #90 locked.

This issue is an actual issue, where the other is not.

iamareebjamal commented 4 years ago

I'm sorry, I cannot have further discussions with a dictator who locks issues as off-topic when confronted with actual arguments while advocating about unrelated technological limitations of SMTP, HTTP APIs, mime types and reliability, none of which I even touched on, nor even mentioned Sorry for wasting your time

amcgregor commented 4 years ago

@iamareebjamal You submit a second issue that duplicates this one passive-aggressively, claim superior knowledge over the codebase than the original author regarding what can and can not be deleted after demonstrating an inability to search properly, then become offended when the duplicate is closed/locked, after the truth of modern e-mail delivery is laid out before you in answer to the "alternatives" question. I really do not know how much more accomodating I could possibly be.

"MIME types" were never mentioned. I do not comprehend your psychology. Good luck, and apologies to the original submitter and additional commentor for being forced to have this play out on their issue. This issue will remain open until I've integrated the removal of marrow.util into develop.

tyoc213 commented 4 years ago

Looks like it's broken on python 3.8.1 currently.

Installs fine:

Indeed it installs fine but importing it causes this

$ python
Python 3.8.2 (default, Mar 26 2020, 15:53:00) 
[GCC 7.3.0] :: Anaconda, Inc. on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from marrow.mailer import Message, Delivery
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/tyoc213/miniconda3/envs/diccionario/lib/python3.8/site-packages/marrow/mailer/__init__.py", line 12, in <module>
    from marrow.mailer.message import Message
  File "/home/tyoc213/miniconda3/envs/diccionario/lib/python3.8/site-packages/marrow/mailer/message.py", line 21, in <module>
    from marrow.mailer.address import Address, AddressList, AutoConverter
  File "/home/tyoc213/miniconda3/envs/diccionario/lib/python3.8/site-packages/marrow/mailer/address.py", line 12, in <module>
    from marrow.util.compat import basestring, unicode, unicodestr, native
  File "/home/tyoc213/miniconda3/envs/diccionario/lib/python3.8/site-packages/marrow/util/compat.py", line 33, in <module>
    from cgi import parse_qsl
ImportError: cannot import name 'parse_qsl' from 'cgi' (/home/tyoc213/miniconda3/envs/diccionario/lib/python3.8/cgi.py)
>>> from marrow.mailer import Mailer, Message
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/tyoc213/miniconda3/envs/diccionario/lib/python3.8/site-packages/marrow/mailer/__init__.py", line 12, in <module>
    from marrow.mailer.message import Message
  File "/home/tyoc213/miniconda3/envs/diccionario/lib/python3.8/site-packages/marrow/mailer/message.py", line 21, in <module>
    from marrow.mailer.address import Address, AddressList, AutoConverter
  File "/home/tyoc213/miniconda3/envs/diccionario/lib/python3.8/site-packages/marrow/mailer/address.py", line 12, in <module>
    from marrow.util.compat import basestring, unicode, unicodestr, native
  File "/home/tyoc213/miniconda3/envs/diccionario/lib/python3.8/site-packages/marrow/util/compat.py", line 33, in <module>
    from cgi import parse_qsl
ImportError: cannot import name 'parse_qsl' from 'cgi' (/home/tyoc213/miniconda3/envs/diccionario/lib/python3.8/cgi.py)

What would be the fastest way to go? for run in python 3.8? Install an older version? (guess not?)

amcgregor commented 4 years ago

@tyoc213 Quick fix while I work on removing marrow.util, isolate that module and patch out the import; being unused, this should have zero impact on marrow.mailer operation. To find the on-disk path to the file to modify pip install e (yup, just the letter e) then run python -me marrow.util.compat. That'll spit out the on-disk path. Edit that file, comment out (or correct) the from cgi line.

tyoc213 commented 4 years ago

Indeed image

It work now, I wait for the

GertBurger commented 4 years ago

Any news on this?

iamareebjamal commented 4 years ago

@GertBurger Author said the change is "trivial" 4 months ago here https://github.com/marrow/mailer/issues/90#issuecomment-617994426

The problem isn't parse* being imported from the wrong location; it's never used within this package. It should be fairly clear the PR would instead need to simply remove marrow.util. It can be outright deleted.

(Bunch → trivial "attribute access dictionary" implementation required, load_object is actually marrow.package:load, boolean is a dictionary lookup w/ fallback, all compat just disappears, and that's… that's basically it.)

I must have misunderstood the meaning of the word.

And he got offended and closed the issue as "off topic" when asked if the project is unmaintained

amcgregor commented 4 years ago

@iamareebjamal In the time it too to write that comment, you could have clicked the pencil icon on GitHub, made the change, and submitted a pull request. Unlimited energy to complain, zero to correct, when the correction has been handed to you. Then there's the non-hilariously trollish claims of dictatorship a) on a site you do not have legally mandated free speech on, b) in a thread ostensibly to gain assistance, and c) on a project you do not own. Even edited your comment to add to the inflammatory. Nice.

What is your goal, really? To irritate and annoy, or actually get something corrected?

If it's the former, let me know; I'll stop resisting the three clicks and advise that such is not an effective means for problem resolution nor conducive towards a positive support environment. For everyone involved, not just you.

iamareebjamal commented 4 years ago

I would have contributed. If the problematic dependency wasn't deprecated and made read only. Un deprecate it and you'll get a PR within hours, I guarantee it. And it'll solve the issue once and for all which has been raised more than 8 months ago.

I don't think I'll make "trivial" changes, which aren't so trivial after all. Why would I contribute to the repo where the author calls proposed changes trivial and can't even make those "trivial" changes themselves. And are offended by the fact that a repo which has got 5 new commits in 4 years and radio silence over such issues is termed as unmaintained. I'll make the changes by the end of the day if you accept the changes aren't so trivial after all. It'll require reimplementing several data types. Or else make the "trivial" changes yourself. But I doubt that's going to happen given that the repo is unmaintained and you want to keep marrow utils deprecated even though directly depending on it.

Either:

  1. Remove the marrow util depreciation and accept a PR there
  2. Don't call proposed changes "trivial" or make those trivial changes yourself
  3. Accept that the project is dead and unmaintained
amcgregor commented 4 years ago

@iamareebjamal From the original issue description, not even the subsequent discussion:

If I go diving into compat.py and change from cgi import parse_qsl to from urllib.parse import parse_qsl, it works fine.

That's a one line change. Almost the definition of trivial. My follow-up of:

Given it's not even used

Means the change isn't even a change, it's only the deletion of lines. Somehow, even more trivial.

  1. Remove the marrow util depreciation and accept a PR there

There may have been a compilation failure in english.c, here.

Don't call proposed changes "trivial" or make those trivial changes yourself.

I haven't? Edited to add to be slightly more helpful to the future on this, but that branch has been in progress for a while, and the scope of the improvements are far more than the removal of one line. It's slow going, making such large changes! (By yourself.)

Accept that the project is dead and unmaintained

Are you really so sure about that? Not sure if wilful blindness to attempt to prove a point, or… actually blind. It's maintained enough to have a butler escort you to the airlock, at least.

ghost commented 4 years ago

That's a one line change. Almost the definition of trivial.

That's what I said. Unarchive marrow.util repo as the code for this "trivial" change is there, not in this repo. I can't possibly make this PR as the project is archived.

Means the change isn't even a change, it's only the deletion of lines. Somehow, even more trivial.

You seem to have problems in your English comprehension skills. Those lines to be deleted are in the archived repo. The changes you're asking to make in this repo are far from trivial. So, please, don't try to pull false equivalences to defend your baseless points.

Unarchive the marrow.util repo and surely enough, the changes will be trivial and I will personally make a PR for it within hours.

But what you want is not trivial at all, so please stop pointing to the 2 lines or deletion, that's what I want to do, you are opposed to that change.

And just like the dictator you are, you blocked me. Can't have people showing facts that you are trying to trivialize the issue by saying that it requires 2 line change but won't allow anyone to make that change, but instead ask for much larger change and appropriate it as trivial without any update on it for months. I can make both changes, I will still do the marrow util change if you unarchive the repo, could have done the larger not so trivial changes in this repo as well if you didn't downplay the changes by saying, jUsT 2 LiNe ChAnGe, JuSt dElEtIonS while advocating for something entirely different. Please stick to one for the sake of legitimacy of your own arguments.

So, again, I'll send the PR to marrow.utils if you unarchive it. I will make the changes required in this repo as well if you stop downplaying the changes by calling them trivial.

PS: The branch you linked is not published. We are talking about the published library. But you would have understood that if your entire objective would be different from shifting goal posts instead of focusing on a crucial bug that some other people are wanting to solve

See how many have forked it to add the trivial fix https://github.com/marrow/util/network It's a shame that the official package can't contain the same trivial fix of 2 lines because you won't unarchive the project

ghost commented 4 years ago

@GertBurger I have switched to this fork https://github.com/LexMachinaInc/mailer https://github.com/LexMachinaInc/util which contains the "trivial" fix we mere mortals are not worthy to enjoy in the official package. I suggest you do the same unless you want to wait another 4 months for a non-update

amcgregor commented 4 years ago

Curiouser and curiouser. That person made the fork, didn't bother opening a PR. Go figure. Edit: and did it wrong. That's just icing on this cavalcade.

ghost commented 4 years ago

They didn't bother opening a PR because the repo is archived. They made correct decision because arguing with a dictator who can't comprehend the reason why people are not able to contribute to his walled garden is because he has archived the project is simply a collosal waste of time. Read the issue description if you're still confused and read it a 100 times more. And continue blaming the contributors and forks while being oblivious to the reason.

amcgregor commented 4 years ago

@areebthrowaway2 Be aware that you are risking your primary account in ban avoidance. You, and your clones, have been reported to GitHub as they arise. The wack-a-mole is un-amusing, and e-mail delivery is one hell of a thing to get a stick up your arse over.

Step back from the keyboard. Take a breath, heck, go for a walk. It'll do you good.

Then learn to read. As pointed out repeatedly, the solution is to remove the dependency on marrow.util, not to re-open and patch the deprecated package. Move on.

See how many have forked it to add the trivial fix…

Uh… so… one? (Debian packaging a fix for this issue does not make.) And not even correctly—"fixing" something that will never get an update. 👍

amcgregor commented 4 years ago

@GertBurger Apologies for the fantastic signal:noise ratio on this issue today. Work is progressing on the next branch which is a complete modernization and update to make use of all modern Python 3 features, including type annotation, modern datatypes, and offers the opportunity for elimination of no longer needed legacy. As a singular developer working on a codebase in such wide use, careful attention is being paid towards making the update as non-breaking as possible.

The "hack and slash" fix (comment out one specific line in your copy) can be effective as a short-term solution, slightly longer-term is the bit of helpful pointer that bubbled out of the chatter, a fork that is patched. However, this is also stop-gap, and an approach entirely taken from the wrong direction.

fiahil commented 4 years ago

Hey everyone,

I ran into this issue today. Rather than switching to a fork, you can temporarily fix this error by adding these lines at the top of the file (before importing Mailer):

import sys  # isort:skip

sys.modules["cgi.parse_qsl"] = None
GertBurger commented 3 years ago

OT but it seems that there is also an issue with Python 3.9 as base64.encodestring() has been removed, should be a trivial fix.

I guess one can do a similar monkey patch to add it back...

amcgregor commented 2 years ago

Looks like my employer paid me to unarchive the repository and patch it. The develop branch of marrow.util successfully operates on Python 3.8 for our SMTP + Mailgun use of Mailer with this patch.

If you are utilizing requirements.txt to pin versions, you can pin marrow.util like this:

-e git+https://github.com/marrow/util.git@cbbc386f8411403953186bf78a9dfcd7cffc42fc#egg=marrow.util

Aside: I also just tested the import process retrieving references to Mailer, MailgunTransport, and SMTPTransport on Python 3.9.

Marking as closed.

amcgregor commented 2 years ago

@GertBurger The encodestring reference was corrected in 32dde28.

davfive commented 2 years ago

Lifesaver!!! Thanks :)

On Thu, Feb 17, 2022 at 1:41 PM Alice Zoë Bevan–McGregor < @.***> wrote:

@GertBurger https://github.com/GertBurger The encodestring reference was corrected in 32dde28 https://github.com/marrow/mailer/commit/32dde282f21513456c933c7ab8bbe55469ec5638 .

— Reply to this email directly, view it on GitHub https://github.com/marrow/mailer/issues/87#issuecomment-1043476811, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGAYW7XL5XNPG2DINQORC3U3VTQLANCNFSM4KLAS3ZQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

Marrin commented 2 years ago

This seems to be patched but there is no release on the Python package index, so the original problem is still there when installing marrow.mailer via pip.

bronikowski commented 10 months ago

I just hit that issue today. It's still buggy on PyPI.