-
Notifications
You must be signed in to change notification settings - Fork 170
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
move calls to get_settings
outside of __init__
#496
move calls to get_settings
outside of __init__
#496
Conversation
@cached_property | ||
def settings(self): | ||
retval = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stevenbal is this going to break our library?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the way our library is implemented now: yes, because we rely on overriding get_settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is too much of a problem? I don't see any changes to that method in particular and instead it will allow us to remove our __init__
override since settings are now consumed lazily. So, what am I missing? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, I skimmed over it too quickly and thought get_settings
was removed here altogether 😅. You're right, this should work and the init override can be removed.
View, middleware and auth backend
Fixes #495