Skip to content

Callables in settings are not verified at start-up if they are strings #1058

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

Closed
iafisher opened this issue May 22, 2023 · 6 comments · Fixed by #1069
Closed

Callables in settings are not verified at start-up if they are strings #1058

iafisher opened this issue May 22, 2023 · 6 comments · Fixed by #1069

Comments

@iafisher
Copy link
Contributor

If you accidentally set, e.g., AXES_LOCKOUT_CALLABLE to an invalid import path, the server will still boot up fine and you won't find out about it until Axes tries to invoke the callable here. In my case, AXES_LOCKOUT_CALLABLE was misconfigured for months before I saw the error in the logs.

It would be ideal if Axes did some kind of verification of the callable settings at start-up. I can think of two ways:

  • Verify settings in conf.py
  • Verify settings in a custom system check

If this seems like a good idea to you I'd be willing to work on it. Not sure if crashing the server on misconfiguration counts as a backwards-incompatible change or not; if so could print a warning instead.

I am using version 5.39.0.

@aleksihakli
Copy link
Member

Thanks for reporting!

This would also be a good opportunity to refactor the callable resolution to use one common function to return one verified callable object from a string path or an object itself. At the moment there is duplicate code in many places and it would be nice to clean that up.

@iafisher and @hirotasoshu would you happen to be interested about improving this and possibly baking a new release as you are familiar with the code base?

@hirotasoshu
Copy link
Contributor

hirotasoshu commented May 27, 2023

@aleksihakli should these checks be in axes/checks.py or is it better to check them immediately in axes/conf.py? Personally, I like the first option more.
One common function to resolve callables is good idea, i will work on this when i have a little more time

@iafisher
Copy link
Contributor Author

@hirotasoshu I'd be happy to write the system check if you want to work on the common function for resolving callables. I can get it done today or tomorrow.

@hirotasoshu
Copy link
Contributor

@hirotasoshu I'd be happy to write the system check if you want to work on the common function for resolving callables. I can get it done today or tomorrow.

Ok, I created #1068 for this

@aleksihakli
Copy link
Member

Thank you for the work on the PR 👍 Upstreamed, I'll bake a new version to PyPI as well.

@aleksihakli
Copy link
Member

Released in version 6.0.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants