rdegges/django-twilio

Django 3.0 support

Closed this issue ยท 19 comments

Just wondering if this is in the works. Thanks :)

jheld commented

Yes. My project is looking to test against 3.0 (currently) and we use this. Assuming tests pass then we'll try some internal testing and go from there.

Thanks for asking!

Awesome, thanks!

jheld commented

We have an open branch, but are waiting for a fix on django dynamic fixtures to have django 3.0 support first.

jheld commented

paulocheque/django-dynamic-fixture#97 is our major blocker right now.

@jheld Anything I could do to help get this merged/tested/released?

jheld commented

@MicahLyle @jimmydess I have cut 0.12.0rc1 and it is now on PyPI. If you can help us determine its stability, that would be great. My project might be a bit longer to find the stability marker.

I can confirm we've had no problems with Twilio's Verification API, and all the relevant imports that come along with that. That's all we're using though on the project where I'm on 0.12.0rc1.

Thanks, @MicahLyle . This is the last package I have before I'm 3.0 compatible, and this upgrade doesn't seem like a big risk to me, so I've been hoping I can avoid changing my Pipfile to get this package upgraded. (I only want to change the lockfile. Pipenv doesn't play nice with prerelease versions, they have to be fully specified in the Pipfile, so then it would stop noticing new updates unless I remember to put the version specifier back once the final release is made).

jheld commented

Gotcha.

Thanks for testing!

We also seem to have stability (on django 2.2 but still!). Yeah the code change in the release was quite small so I didn't anticipate issues arising.

I will plan to cut the official release either this weekend or by early next week.

I do also try to make sure to make a release on the github project page. I do try to update issues/pull requests to help folks but if you happen to subscribe to the release changes (does github let you do that?), that likely will be more consistent.

I do try to update issues/pull requests to help folks but if you happen to subscribe to the release changes (does github let you do that?), that likely will be more consistent.

Subscribing to releases can be good. But my issue isn't really about notifying me, its about messing with my automation, and my workflow. It's not just about passing the release to me, a human, its about making sure that my automated processes will see it, and help me remember when I as a human often will forget (even when I'm reminded). This is somewhat a shortcoming in Pipenv, to be sure.

However, and please understand that I'm a maintainer of other projects and I'm attempting to give friendly, not annoying, criticism, I might suggest that you're being a bit over-cautious about these releases. Unless I've misunderstood what this latest release was about, it was just adding a version specifier that stopped blocking this package from being installed alongside Django 3. It didn't change any significant code, or even fix bugs. The new version, if it had been released, would not have been able to break any existing code. It's only code that chose to upgrade that might have had an issue, and that strikes me as perfectly reasonable.

What I do in my own projects, and what I'd suggest to you here, is to select a minimum version of Django you support, but to not limit future compatibility. That way, when a new version of Django is released, you won't need to do any adjustments or releases, unless that new version of Django actually breaks your code. It very well may allow for less code churn in this project, even as new Django versions are released.

OK, enough of my unsolicited but hopefully friendly advice. I hope that it makes you think that I'm trying to make your life a bit easier as a maintainer (which I am), and not merely trying to make my life as a consumer easier (though that is also true).

Whatever you decide to do, thank you very much for the work that you have put into this library, and that you continue to put into maintaining it. Your work has added real, tangible benefit to my work, and I am totally sure it has done the same for many others in our community.

jheld commented

@ryanhiebert Oh, I'm definitely over cautious. Part of that is that like most maintainers I do not tend to spend a lot of time (or get to) on keeping my OSS projects up to date. I try to ensure that the test suite is valid and that the common cases are covered/usable/don't break. And if my company/project doesn't use the target change (django 3.0, we aren't there just yet), then I'm even more cautious, but the rc release at least gives people a chance to help test.

Side note re:Pip lock/env/poetry, I've never used that tech; I do sympathize with the frustration/concern, though.

Unfortunately there was a change in this one because django 3.0 removed their six vendored package. To be clear, it's not so much a blame game as much as they made a decision, and I not thinking too much about it, did the minimal effort required for django 2.0 to work, which is why 3.0 broke -- it was a remaining vestige of python2.7 "support" being removed. Had I remove those references earlier then this would have been a hot swap.

As far as the version clipping is concerned, yes, there is room for improvement. I'm not likely going to get better at it in the short term, but yes, I'd like to have that happen.

By the same token, I'd still rather have an alpha beta or rc release before basically any fix/change goes live (I realize that is indeed a bit over-doing it). It does give a little bit of extra issues around humans and automation, but it also helps further reduce the off-chance of something breaking production code (my project or downstream projects that rely on this).

That being said, again, I do appreciate your write-up and suggestions.

And...0.12.0 is now on PyPI! Release has been cut.

jheld commented

If anyone does find issues with this django 3.0 integration, please open a new ticket with as many details as possible. Given that this was supposed to only be import six/bytes/str simple line changes, though, I don't anticipate there really being any issues. But, do not hesitate! Closing this issue.

Whelp, @jheld, it looks like it still can't install, because the version of Django is still being limited to <3.0 in all cases. https://github.com/rdegges/django-twilio/blob/master/setup.py#L25

Which just means that, even though you fixed the code so it can run Django 3.0, the setup.py file isn't updated to allow me to even try it.

jheld commented

Gotcha. Sorry about that. I'll try to fix that tomorrow morning. 0.12.1, probably. I'll likely delete 0.12.0 since though it works for some, it's certainly misleading at best.

I'm more annoyed that CI was working (there is a pypy 3 issue which seems unrelated). Had those tests broken on the rc I would have fixed. Curious how/what was actually installed for the django3 env marker...

Anyway, thank you!

No problem, I appreciate the time you're putting in to getting this out. Thank you!

jheld commented

Thank you for helping out!

@ryanhiebert
Before I publish 0.12.1 to PyPI, would you be able to test it out on your side? I've pushed it to master and made a tag already.

I can confirm that it is allowing it to install Django 3.0.2.

Secondarily, what will it take for you to be willing to add Django 3.1 to the version restriction? At the alpha or beta?

jheld commented

i'll put this one out as is. I accidentally "rushed" this partial fix and so no rc for 0.12.1. as you say, it was really just a setup.py issue so changing the django req inside it was just about more granularity per-version.

As soon as 3.1 is bound to come out, I'll make either a 0.12.2 or a 0.13.0, depending on our change-set.

I'd like to explicitly cut out the unsupported/incompatible versions in the next release or two, so that will also shave off some of the setup.py complexity (and let the requirements txt be a little more up to date, too).

OK, it's on PyPI! Please use 0.12.1