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

Support for Django 5.0 and python 3.12 + Fix CI #277

Merged
merged 4 commits into from
Dec 10, 2023

Conversation

pfouque
Copy link
Collaborator

@pfouque pfouque commented Dec 2, 2023

  • Add Django 3.2, 4.0, 4.1, 4.2, 5.0 support

  • Remove support for deprecated Django versions

  • Add Python 3.8, 3.9, 3.10, 3.11, 3.12 support

  • Remove support for deprecated Python versions

  • Fix CI

@pfouque pfouque changed the title Support for Django 5.0 and python 3.12 Support for Django 5.0 and python 3.12 + Fix CI Dec 3, 2023
tox.ini Outdated Show resolved Hide resolved
Copy link
Collaborator

@Pietro395 Pietro395 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testend with Django 4 and POP3, for now everything is ok, in the next few days we will try Office365

@apmathento
Copy link

Hello! How long will it take aprox? Thank you! :)

@Pietro395
Copy link
Collaborator

Hello! How long will it take aprox? Thank you! :)

If your comment refers to me I think I'll try it between tomorrow and the day after tomorrow

@coddingtonbear
Copy link
Owner

Thanks for helping out by testing this -- I really, really appreciate the effort! Assuming you don't discover any interesting problems, and you or somebody else checks on a few things I've noted below, I can commit to pulling a release together in the next few days.

There are just a few things I think might need some attention from you or anybody else who might have a little time:

  1. Verifying the processincomingmessage e-mail ingestion management command. I think that would be just as easy as cat-ing an e-mail message to python manage.py processingomingmessage SOMEMAILBOXNAME where SOMEMAILBOXNAME is literally any string (it'll create a mailbox with that name if it doesn't already exist) and your e-mail message is raw e-mail text (see https://github.com/coddingtonbear/django-mailbox/tree/master/django_mailbox/tests/messages for some options, but, really, anything will do).
  2. Verifying the Office365 integration. You've said you're planning to look into this already, @Pietro395.
  3. I assume there are no backward-incompatible changes around this, but if there are, somebody let me know so I can be sure to increment the version number's major digit as necessary.

I mentioned above that once the above items are looked into I can pull together a release, but I do have to warn you all that I have some time constraints around this in that I'll be unavailable for several weeks starting next week; so if we're hoping to have a release before mid-January, I'm afraid there's a little bit of a crunch. If some folks can confirm the things noted above before midnight GMT on Friday night (8 December), I'm confident I can get the release sorted out, though.

@Pietro395
Copy link
Collaborator

1. Verifying the `processincomingmessage` e-mail ingestion management command.  I think that would be just as easy as `cat`-ing an e-mail message to `python manage.py processingomingmessage SOMEMAILBOXNAME` where `SOMEMAILBOXNAME` is literally any string (it'll create a mailbox with that name if it doesn't already exist) and your e-mail message is raw e-mail text (see https://github.com/coddingtonbear/django-mailbox/tree/master/django_mailbox/tests/messages for some options, but, really, anything will do).

(env) [intacloud@intacloud-test django-struttura]$ cat /home/intacloud/django-mailbox/django_mailbox/tests/messages/message_with_attachment.eml | python3 manage.py processincomingmessage HELLOWORLD2
INFO 2023-12-05 21:57:54,103 Message received from Adam Coddington <test@adamcoddington.net>

Looks good to me!!

@Pietro395
Copy link
Collaborator

2. Verifying the Office365 integration.  You've said you're planning to look into this already, @Pietro395.

I tested the integration with Office365 by authorizing a new tenant and trying to receive emails.
I haven't encountered any problems, everything seems to work fine

(env) [intacloud@intacloud-test django-struttura]$ python3 manage.py getmail
INFO 2023-12-06 17:37:12,397 Gathering messages for HELLOWORLD
INFO 2023-12-06 17:37:12,397 Gathering messages for HELLOWORLD2
INFO 2023-12-06 17:37:12,397 Gathering messages for donotreply
INFO 2023-12-06 17:37:12,580 Gathering messages for admin_office365
INFO 2023-12-06 17:37:13,612 Received Test for Office365 (from ['pietro@xxxx.it'])

immagine

@coddingtonbear I believe you can create the release 🚀

@coddingtonbear coddingtonbear merged commit 7fde5f7 into coddingtonbear:master Dec 10, 2023
@coddingtonbear
Copy link
Owner

Released as part of 4.9.0.

I have to say, though, that I couldn't get the tests running in the short period of time I had to look into this -- apparently many of the testing dependencies are out-of-date.

I wonder if I could trouble any of you to see if you might be able to get the tests running... because it sure feels pretty weird to me to push up code without running tests, but I understand that getting a release together is important to some of you so I've decided to, I guess, cross my fingers that nothing unexpected is broken?

It should be just as easy as installing test dependencies in test_requirements.txt and running pytest, if I remember correctly, but it has honestly been a very long time since I've run tests on this particular project.

@pfouque
Copy link
Collaborator Author

pfouque commented Dec 10, 2023

Thanks a lot @coddingtonbear ( and @Pietro395 for your support)
I did run the tests using tox and it was all good.
I even fixed the CI, but I made a small mistake with the name of the master branch in the workflow file. If we replace 'main' by 'master' it should run on the main branch (in addition to the pull requests).
Now we can consider the project is back on track, and a bit of triaging in the PR/issues would be nice.
Maybe we could have some labels to identify the outdated ones before closing them.

@Pietro395
Copy link
Collaborator

Thanks a lot @coddingtonbear ( and @Pietro395 for your support)
I did run the tests using tox and it was all good.
I even fixed the CI, but I made a small mistake with the name of the master branch in the workflow file. If we replace 'main' by 'master' it should run on the main branch (in addition to the pull requests).
Now we can consider the project is back on track, and a bit of triaging in the PR/issues would be nice.
Maybe we could have some labels to identify the outdated ones before closing them.

Thanks for your contribution @pfouque and for your time @coddingtonbear 😊, I also think we should tag the old issues to start from scratch

@coddingtonbear
Copy link
Owner

coddingtonbear commented Dec 11, 2023

Hey there @pfouque & @Pietro395 -- You've already noticed that I've added you both as maintainers on this project. I did that because you have both already put forth more more than enough effort to demonstrate that I can trust you both to make good choices around things. Don't feel like I'm putting an obligation on you, though -- of all people, I can understand that you probably have other priorities, but I do thank you for any help you can provide!

As far as guidelines, I think in general the following sound reasonable to me:

  • I trust you both to merge things that are obviously safe and unlikely to break anything -- ideally after it has been reviewed by one maintainer (other than whoever wrote the original PR, in the case of one of us making a PR), but please try your best to not break the tests. They don't offer us a huge amount of safety on this particular project, but they're better than nothing, and ideally we'd have more rather than fewer.
  • Ideally PRs adding features should be weighed against the long-term maintenance overhead that they might add. In my experience, one of the main things that kills projects like this one is adoption of complicated, niche features that few people use and understand the intent of well enough to understand what other changes might break it. If you decide to merge a new feature, please think about how you (or somebody else) might support that feature years down the road -- very often the original contributors disappear after the initial PR; so you can't rely on them to support it.
  • For the moment, I can handle the releases for us, but eventually, I think it's probably reasonable to wire things up to github actions (assuming we end up having enough releases for that to make sense). Just let me know when you think you've added a feature worth releasing and (for semver reasons), whether that release includes any breaking changes (no matter how minor), a new feature, or just a fix for an existing feature.

We should probably set up a channel for just normal conversations about things, though. There's already a public gitter channel for this project, but maybe we should exchange contact info so we can figure out a place for us to chat about things privately? I already have your e-mail, @Pietro395, but maybe you could e-mail me, @pfouque so I can start an e-mail conversation among the three of us to figure out where we'd like that chat to be?

Cheers, and thank you so much for all of your help so far!

@pfouque pfouque deleted the dj5_py312 branch December 11, 2023 05:14
@pfouque
Copy link
Collaborator Author

pfouque commented Dec 17, 2023

Hi @coddingtonbear,
Thanks a lot for that; it's warmly welcomed!

maybe you could e-mail me, @pfouque so I can start an e-mail conversation among the three of us to figure out where we'd like that chat to be?

I suppose you have access to it now that I accepted the invite.
TBH, I'm not sure to follow gitter as a new canal, but I'm fine with email or we could have a issue-only repository to hold the private discussion all in Github.

For the moment, I can handle the releases for us

👍 The last release is already a great achievement!
And IMO the scope of the project is "small" enough so that it should not require any monthly release plan.

please try your best to not break the tests.

I fixed the CI, especially for that! ;)

Ideally PRs adding features should be weighed against the long-term maintenance overhead that they might add

(I'm applying the same rule to my corporate project contributions! 😜)

very often the original contributors disappear after the initial PR

About that, we have plenty of stalled issues and PRs...making the project look "unmaintained".
Since the project is now back on track (with currently supported versions of Django),it should be easier to welcome new contributors.
I would consider triaging as a priority, especially the items without any PR, evidence, or useful information.
Even for those who have all of this can be tedious to diagnose whether the problem has been resolved in the meantime or not.
The fact is that most of the issues are on a now deprecated version, so I would vote for a pretty aggressive cleaning after some basic investigation (contacting a 3-year-old contributor has little chance of success).

emilyh2 added a commit to factal-inc/django-mailbox that referenced this pull request Feb 14, 2024
* Fix message/rfc822 attachment processing

* Update models.py, __init__.py, and office365.py

* Update setup.py

* Office365Transport properties

* Update .gitignore

* wrong folder stage

* Update models.py

* Update office365.py and models.py

* office365 implementation

* Update office365.py

* clear code

* fix import

* Update readme.rst

* archive and delete

* fix and documentation

* removed build folder

* missing archive

* Office365 API mailbox support (coddingtonbear#251)

* Update models.py, __init__.py, and office365.py

* Update setup.py

* Office365Transport properties

* Update .gitignore

* wrong folder stage

* Update models.py

* Update office365.py and models.py

* office365 implementation

* Update office365.py

* clear code

* fix import

* Update readme.rst

* archive and delete

* fix and documentation

* removed build folder

* missing archive

Co-authored-by: Pietro Mingo <p.mingo@intac.it>
Co-authored-by: Serafim Bordei <s.bordei@intac.it>

* updating location of logic for getmail command (coddingtonbear#264)

* moving logic for processing all the new emails from all mailboxes to a method under Mailbox so it can be called from other modules

* moving logger lines into handle method as requested

* Update utils.py

Fixing DJANGO_MAILBOX_default_charset error, it does not detect the settings in django correctly because one part is in lower case.
Changed to DJANGO_MAILBOX_DEFAULT_CHARSET.

* Initial Bulgarian translation

* Close attachments

* Support for Django 5.0 and python 3.12 + Fix CI (coddingtonbear#277)

* Upgrade to last Django & Python supported versions

* flake8 fixes

* Add CI

* Add final version of Django 5.0

* Bump version: 4.8.2 → 4.9.0

* fix CI implemented by pfouque

* Change to documentation regarding Office365 configuration

* Remove south migrations

* cleanup and update

* Fix DJANGO_MAILBOX_DEFAULT_CHARSET setting

* Make eml field Blankable

* MailboxAttachment __str__ change

* Update django_mailbox/models.py

Co-authored-by: Pascal Fouque <pfouque@users.noreply.github.com>

* Remove default_app_config

---------

Co-authored-by: Miłosz Jerkiewicz <milosz.jerkiewicz@baumeister-rosing.de>
Co-authored-by: Pietro Mingo <p.mingo@intac.it>
Co-authored-by: Pietro Mingo <pietro.mingo@gmail.com>
Co-authored-by: Serafim Bordei <s.bordei@intac.it>
Co-authored-by: Pietro Mingo <50447537+skar395@users.noreply.github.com>
Co-authored-by: Jace Manshadi <jmanshad@protonmail.com>
Co-authored-by: Robersillo <31243904+Robersillo@users.noreply.github.com>
Co-authored-by: Arneatec <nouser@nodomain.com>
Co-authored-by: Martin Manzo <martinmanzo@users.noreply.github.com>
Co-authored-by: Pietro <me@pietro.in>
Co-authored-by: Pascal F <pfouque@users.noreply.github.com>
Co-authored-by: Adam Coddington <me@adamcoddington.net>
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

Successfully merging this pull request may close these issues.

4 participants