Add missing dependency and retool settings for modern Django#61
Add missing dependency and retool settings for modern Django#61itsthejoker wants to merge 9 commits intothread:masterfrom
Conversation
|
I was thinking about it today and this does introduce one breaking change: the undocumented SITE_URL setting now needs to be LIGHTWEIGHT_QUEUE_SITE_URL, though I don't know how important that is to you. |
|
@PeterJCLaw @alexivanov Hey there -- any thoughts on this PR? I'm happy to adjust anything needed. |
|
Hi @itsthejoker, Thanks for your contribution, this looks like a really useful set of changes :) There's a lot going on in this PR and I expect it will be easier to discuss it if it were broken into separate parts (the parts you identified in the PR description seem like good boundaries). Would you be up for doing that? One change I have pulled in already was sorting the missing dependency; thanks for raising that. I'm guessing that that crept in when we added type annotations in v4.1. I've released the fix as v4.5.1, so you may also want to rebase on top of that. Thanks, |
|
@PeterJCLaw I spread the changes out across three PRs -- one stacked set of two and a standalone. Closing this one. |
Hi, all!
Quick overview:
typing_extensionspackagedebug_webThe Big Kahuna
Missing package & updates
I grabbed this package from PyPI for a project I'm working on, but on installing it throws an error because
typing_extensionsisn't installed. I found that's becausetyping_extensionsis defined insidepoetry.lock, but not actually listed as a dependency inside yourpyproject.tomlfile -- so any time you install using thepoetry.lockfile it'll work fine, but any build created will be missing that dependency since only the information inpyproject.tomlgets copied over tosetup.py.This PR adds
typing_extensionsas an explicit dependency so that it will appear in future builds and also just generally runspoetry update.Converting the Settings
This is what the bulk of this PR is concerned with; the existing system for settings assumes that everything is going to be in a single file with everything already defined. If you use a settings routing pattern like I do (where the root settings is just "if ENVIRONMENT == 'prod': from settings.base import *; if ENVIRONMENT == 'staging': from settings.staging import *") then it errors out on every key because nothing is technically available yet.
I rebuilt the settings to match the more flexible "get the setting when we actually need it" design pattern, taking inspiration from django-cors-headers and how that works. This one is slightly more complicated because
django-lightweight-queueexpects to be able to overwrite every value at runtime, so it retains that ability.Deprecations from Django 3
from django.conf.urls import urlhas been removed in Django 4, so I've substitutedre_pathwhich functions the same way and is valid in versions 2 through 4.Various fixes for Debug Web
Debug Web has two major issues: the first is that the reverse is looking for something that is not the app name (
django-lightweight-queuevsdjango_lightweight_queue) and the second is that it expects a setting that is not standard and not listed anywhere else:SITE_URL. I've updated the reverse and added documentation about SITE_URL along with making it available as a custom setting for the package.Updated Readme
Added a few sections to the README relating to setting up
django_lightweight_queuethat were things I needed to know but were only discoverable by trawling through the source code. Hopefully it will help others who want to use this package.I apologize for dropping an overly large PR on you over the weekend; I tried to be extremely careful in retaining existing expected functionality while still upgrading where necessary. I'm happy to update if you have requests -- as it stands, it passes flake8, isort, and the testing suite.
Cheers!