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

Add ruff format & lint (isort only) #560

Merged
merged 1 commit into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,25 @@ name: Test
on: [push, pull_request]

jobs:
ruff-format:
runs-on: ubuntu-latest
timeout-minutes: 1
steps:
- uses: actions/checkout@v4
- uses: chartboost/ruff-action@v1
with:
version: 0.5.0
args: 'format --check'

ruff-lint:
runs-on: ubuntu-latest
timeout-minutes: 1
steps:
- uses: actions/checkout@v4
- uses: chartboost/ruff-action@v1
with:
version: 0.5.0

build:
runs-on: ubuntu-latest
strategy:
Expand Down
1 change: 1 addition & 0 deletions constance/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
class LazyConfig(LazyObject):
def _setup(self):
from .base import Config

self._wrapped = Config()


Expand Down
46 changes: 18 additions & 28 deletions constance/admin.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
from collections import OrderedDict
from datetime import date, datetime
from datetime import date
from datetime import datetime
from operator import itemgetter

from django import forms, get_version
from django import forms
from django import get_version
from django.apps import apps
from django.contrib import admin, messages
from django.contrib import admin
from django.contrib import messages
from django.contrib.admin.options import csrf_protect_m
from django.core.exceptions import PermissionDenied
from django.http import HttpResponseRedirect
Expand All @@ -13,7 +16,8 @@
from django.utils.formats import localize
from django.utils.translation import gettext_lazy as _

from . import LazyConfig, settings
from . import LazyConfig
from . import settings
from .forms import ConstanceForm
from .utils import get_values

Expand All @@ -31,12 +35,8 @@ def __init__(self, model, admin_site):
def get_urls(self):
info = self.model._meta.app_label, self.model._meta.module_name
return [
path('',
self.admin_site.admin_view(self.changelist_view),
name='%s_%s_changelist' % info),
path('',
self.admin_site.admin_view(self.changelist_view),
name='%s_%s_add' % info),
path('', self.admin_site.admin_view(self.changelist_view), name='%s_%s_changelist' % info),
path('', self.admin_site.admin_view(self.changelist_view), name='%s_%s_add' % info),
]

def get_config_value(self, name, options, form, initial):
Expand Down Expand Up @@ -88,9 +88,7 @@ def changelist_view(self, request, extra_context=None):
form_cls = self.get_changelist_form(request)
form = form_cls(initial=initial, request=request)
if request.method == 'POST' and request.user.has_perm('constance.change_config'):
form = form_cls(
data=request.POST, files=request.FILES, initial=initial, request=request
)
form = form_cls(data=request.POST, files=request.FILES, initial=initial, request=request)
if form.is_valid():
form.save()
messages.add_message(
Expand All @@ -117,9 +115,7 @@ def changelist_view(self, request, extra_context=None):
django_version=get_version(),
)
for name, options in settings.CONFIG.items():
context['config_values'].append(
self.get_config_value(name, options, form, initial)
)
context['config_values'].append(self.get_config_value(name, options, form, initial))

if settings.CONFIG_FIELDSETS:
if isinstance(settings.CONFIG_FIELDSETS, dict):
Expand All @@ -136,24 +132,18 @@ def changelist_view(self, request, extra_context=None):
fields_list = fieldset_data
collapse = False

absent_fields = [field for field in fields_list
if field not in settings.CONFIG]
absent_fields = [field for field in fields_list if field not in settings.CONFIG]
assert not any(absent_fields), (
"CONSTANCE_CONFIG_FIELDSETS contains field(s) that does "
"not exist: %s" % ', '.join(absent_fields))
'CONSTANCE_CONFIG_FIELDSETS contains field(s) that does ' 'not exist: %s' % ', '.join(absent_fields)
)

config_values = []

for name in fields_list:
options = settings.CONFIG.get(name)
if options:
config_values.append(
self.get_config_value(name, options, form, initial)
)
fieldset_context = {
'title': fieldset_title,
'config_values': config_values
}
config_values.append(self.get_config_value(name, options, form, initial))
fieldset_context = {'title': fieldset_title, 'config_values': config_values}

if collapse:
fieldset_context['collapse'] = True
Expand Down Expand Up @@ -209,4 +199,4 @@ def label_lower(self):
_meta = Meta()


admin.site.register([Config], ConstanceAdmin)
admin.site.register([Config], ConstanceAdmin)
1 change: 0 additions & 1 deletion constance/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,3 @@ class ConstanceConfig(AppConfig):

def ready(self):
from . import checks

1 change: 0 additions & 1 deletion constance/backends/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@


class Backend:

def get(self, key):
"""
Get the key from the backend store and return the value.
Expand Down
30 changes: 15 additions & 15 deletions constance/backends/database.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
from django.core.cache import caches
from django.core.cache.backends.locmem import LocMemCache
from django.core.exceptions import ImproperlyConfigured
from django.db import (
IntegrityError,
OperationalError,
ProgrammingError,
transaction,
)
from django.db import IntegrityError
from django.db import OperationalError
from django.db import ProgrammingError
from django.db import transaction
from django.db.models.signals import post_save

from constance import config
from constance import settings
from constance import signals
from constance.backends import Backend
from constance import settings, signals, config


class DatabaseBackend(Backend):
def __init__(self):
from constance.models import Constance

self._model = Constance
self._prefix = settings.DATABASE_PREFIX
self._autofill_timeout = settings.DATABASE_CACHE_AUTOFILL_TIMEOUT
Expand All @@ -24,24 +25,25 @@ def __init__(self):
if self._model._meta.app_config is None:
raise ImproperlyConfigured(
"The constance.backends.database app isn't installed "
"correctly. Make sure it's in your INSTALLED_APPS setting.")
"correctly. Make sure it's in your INSTALLED_APPS setting."
)

if settings.DATABASE_CACHE_BACKEND:
self._cache = caches[settings.DATABASE_CACHE_BACKEND]
if isinstance(self._cache, LocMemCache):
raise ImproperlyConfigured(
"The CONSTANCE_DATABASE_CACHE_BACKEND setting refers to a "
'The CONSTANCE_DATABASE_CACHE_BACKEND setting refers to a '
"subclass of Django's local-memory backend (%r). Please "
"set it to a backend that supports cross-process caching."
% settings.DATABASE_CACHE_BACKEND)
'set it to a backend that supports cross-process caching.' % settings.DATABASE_CACHE_BACKEND
)
Comment on lines 34 to +38
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we take the opportunity to migrate this to a f-string?

Copy link
Member Author

Choose a reason for hiding this comment

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

We should. There are linters for that in ruff. But to reduce complexity of reviewing process I've moved this out of scope of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the start of a great refresh 🙏

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. than go next 😄

else:
self._cache = None
self.autofill()
# Clear simple cache.
post_save.connect(self.clear, sender=self._model)

def add_prefix(self, key):
return f"{self._prefix}{key}"
return f'{self._prefix}{key}'

def autofill(self):
if not self._autofill_timeout or not self._cache:
Expand Down Expand Up @@ -114,9 +116,7 @@ def set(self, key, value):
if self._cache:
self._cache.set(key, value)

signals.config_updated.send(
sender=config, key=key, old_value=old_value, new_value=value
)
signals.config_updated.send(sender=config, key=key, old_value=old_value, new_value=value)

def clear(self, sender, instance, created, **kwargs):
if self._cache and not created:
Expand Down
8 changes: 4 additions & 4 deletions constance/backends/memory.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
from threading import Lock

from .. import config
from .. import signals
from . import Backend
from .. import signals, config


class MemoryBackend(Backend):
"""
Simple in-memory backend that should be mostly used for testing purposes
"""

_storage = {}
_lock = Lock()

Expand All @@ -33,6 +35,4 @@ def set(self, key, value):
with self._lock:
old_value = self._storage.get(key)
self._storage[key] = value
signals.config_updated.send(
sender=config, key=key, old_value=old_value, new_value=value
)
signals.config_updated.send(sender=config, key=key, old_value=old_value, new_value=value)
18 changes: 9 additions & 9 deletions constance/backends/redisd.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
from pickle import loads, dumps
from pickle import dumps
from pickle import loads
from threading import RLock
from time import monotonic

from django.core.exceptions import ImproperlyConfigured

from .. import config
from .. import settings
from .. import signals
from .. import utils
from . import Backend
from .. import settings, utils, signals, config


class RedisBackend(Backend):

def __init__(self):
super().__init__()
self._prefix = settings.REDIS_PREFIX
Expand All @@ -20,15 +23,14 @@
try:
import redis
except ImportError:
raise ImproperlyConfigured(
"The Redis backend requires redis-py to be installed.")
raise ImproperlyConfigured('The Redis backend requires redis-py to be installed.')

Check warning on line 26 in constance/backends/redisd.py

View check run for this annotation

Codecov / codecov/patch

constance/backends/redisd.py#L26

Added line #L26 was not covered by tests
if isinstance(settings.REDIS_CONNECTION, str):
self._rd = redis.from_url(settings.REDIS_CONNECTION)
else:
self._rd = redis.Redis(**settings.REDIS_CONNECTION)

def add_prefix(self, key):
return f"{self._prefix}{key}"
return f'{self._prefix}{key}'

def get(self, key):
value = self._rd.get(self.add_prefix(key))
Expand All @@ -47,9 +49,7 @@
def set(self, key, value):
old_value = self.get(key)
self._rd.set(self.add_prefix(key), dumps(value, protocol=settings.REDIS_PICKLE_VERSION))
signals.config_updated.send(
sender=config, key=key, old_value=old_value, new_value=value
)
signals.config_updated.send(sender=config, key=key, old_value=old_value, new_value=value)


class CachingRedisBackend(RedisBackend):
Expand Down
7 changes: 4 additions & 3 deletions constance/base.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
from . import settings, utils
from . import settings
from . import utils


class Config:
"""
The global config wrapper that handles the backend.
"""

def __init__(self):
super().__setattr__('_backend',
utils.import_module_attr(settings.BACKEND)())
super().__setattr__('_backend', utils.import_module_attr(settings.BACKEND)())

def __getattr__(self, key):
try:
Expand Down
31 changes: 14 additions & 17 deletions constance/checks.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
from typing import Tuple, Set, List
from typing import List
from typing import Set
from typing import Tuple

from django.core import checks
from django.core.checks import CheckMessage
from django.utils.translation import gettext_lazy as _


@checks.register("constance")
@checks.register('constance')
def check_fieldsets(*args, **kwargs) -> List[CheckMessage]:
"""
A Django system check to make sure that, if defined,
Expand All @@ -14,28 +17,22 @@ def check_fieldsets(*args, **kwargs) -> List[CheckMessage]:

errors = []

if hasattr(settings, "CONFIG_FIELDSETS") and settings.CONFIG_FIELDSETS:
if hasattr(settings, 'CONFIG_FIELDSETS') and settings.CONFIG_FIELDSETS:
missing_keys, extra_keys = get_inconsistent_fieldnames()
if missing_keys:
check = checks.Warning(
_(
"CONSTANCE_CONFIG_FIELDSETS is missing "
"field(s) that exists in CONSTANCE_CONFIG."
),
hint=", ".join(sorted(missing_keys)),
obj="settings.CONSTANCE_CONFIG",
id="constance.E001",
_('CONSTANCE_CONFIG_FIELDSETS is missing ' 'field(s) that exists in CONSTANCE_CONFIG.'),
hint=', '.join(sorted(missing_keys)),
obj='settings.CONSTANCE_CONFIG',
id='constance.E001',
)
errors.append(check)
if extra_keys:
check = checks.Warning(
_(
"CONSTANCE_CONFIG_FIELDSETS contains extra "
"field(s) that does not exist in CONFIG."
),
hint=", ".join(sorted(extra_keys)),
obj="settings.CONSTANCE_CONFIG",
id="constance.E002",
_('CONSTANCE_CONFIG_FIELDSETS contains extra ' 'field(s) that does not exist in CONFIG.'),
hint=', '.join(sorted(extra_keys)),
obj='settings.CONSTANCE_CONFIG',
id='constance.E002',
)
errors.append(check)
return errors
Expand Down
2 changes: 1 addition & 1 deletion constance/context_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ def config(request):
)

"""
return {"config": constance.config}
return {'config': constance.config}
Loading