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 #467

Open
estahn opened this issue Aug 29, 2023 · 8 comments
Open

Support for Django #467

estahn opened this issue Aug 29, 2023 · 8 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@estahn
Copy link

estahn commented Aug 29, 2023

Is your feature request related to a problem? Please describe.

We use aws-xray-sdk with our Django app.

deptry reports:

pyproject.toml: DEP002 'aws-xray-sdk' defined as a dependency but not used in the codebase

I assume the reason for this is that the module is only referenced as part of the INSTALLED_APPS setting:

    INSTALLED_APPS: List[str] = [
        ...
        "aws_xray_sdk.ext.django",
		...
    ]

Describe the solution you would like

I have seen the following used in other projects:

[tool.django-stubs]
django_settings_module = "app.settings"

This could possibly be adopted:

[tool.deptry]
django_settings_module = "app.settings"

Alternatively could code coverage reports be used to complete a better picture on what is executed?

@estahn estahn added the enhancement New feature or request label Aug 29, 2023
@anentropic
Copy link

I'm looking for the same feature, haven't found any unused dependency checker yet which is pluggable enough to add a special case for Django INSTALLED_APPS deferred framework imports without hacking it into the checker itself

@fpgmaas
Copy link
Owner

fpgmaas commented Oct 2, 2023

Thanks for the feedback! I have not used Django myself, but I am assuming what needs to happen for this;

  • Add an argument django_settings_module/django-settings-module to cli.py and core.py
  • Create a class that parses the INSTALLED_APPS from the django-settings-module file.
  • And then here use that to modify the following line
for module, locations in get_imported_modules_for_list_of_files(all_python_files).items()

into something like

for module, locations in (
   get_imported_modules_for_list_of_files(all_python_files) + 
   get_imported_modules_for_django(django_settings_module)
   ).items()

Although that line and the lines around it might need some refactoring because it has become quite a long list comprehension.

I am a bit busy at this time, but if I have time in the near future I will try to get around to this. I will also tag this with 'Good first issue', so if someone else needs it earlier or feels like contributing, I hope the above lines can give some idea on how to get started.

@fpgmaas fpgmaas added good first issue Good for newcomers help wanted Extra attention is needed labels Oct 2, 2023
@mkniewallner
Copy link
Collaborator

Not at all against implementing that, but depending on how the settings definition is implemented in a Django project, INSTALLED_APPS could sometimes be defined once in a file, then overrode later on in the same file, or in multiple files, if for instance you want to append some apps only on a dev environment.

So this would probably only be implemented as a "best effort", should we go with parsing the attribute with AST, given how dynamic this could be. Ideally, we would resolve the attribute through an import, but this is not something we would want to do, as we don't want to tie deptry to runtime resolution of parsed Python code.

@anentropic
Copy link

Yes I think it's not solvable in most general case without importing Django settings file, I can see why you wouldn't want to do that.

In which case maybe my solution would be something like a custom Django manage.py command which parses the INSTALLED_APPS and updates the deptry config to make exceptions for them.

It wouldn't have to be part of deptry. Could be run automatically as a pre-commit hook.

It would need a way to map from import names back to PyPI package names, but I guess that exists somewhere in deptry already that it could borrow.

@Wonskcalb
Copy link

Wonskcalb commented Jun 3, 2024

I'd be very interested in such a feature as well, since deptry indeed shows some false positives when used on a Django project.

Although I'm not currently in the capacity to participate on the technical discussion for this support, I believe there is at least a couple more settings that should be mentioned: In Django, Databases and caches configuration are made through the DATABASES and CACHES settings, with a more complex format, looking as follows

CACHES = {
    "default": {
        "BACKEND": "django_redis.cache.RedisCache",  # String representing module path
        "LOCATION": REDIS_URL,
        "OPTIONS": {"CLIENT_CLASS": "django_redis.client.DefaultClient"},
    }
}

Edit: Added the rest of my message... I pressed enter too soon 🙈.

@uptickmetachu
Copy link

Can we use a special function that adds the dot syntax imports into a list of registered imports?

def register_import_string(dotted_import: str) -> str:
    some_import_registrar.add(dotted_import)
    return dotted_import

Replace all dot imports in the code with register_import_string("django_redis.cache.RedisCache")

The benefit is that deptry doesn't need to support django specifically and it is generic enough to apply anywhere.

@violuke
Copy link

violuke commented Oct 7, 2024

We would also love to see this 🙏

@nifadyev
Copy link

In which case maybe my solution would be something like a custom Django manage.py command which parses the INSTALLED_APPS and updates the deptry config to make exceptions for them.

Hey @anentropic , what about making optional django dependency and use django settings only if necessary? In that case, current logic is unaffected and proper django support is available via deptry[django]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

8 participants