allow swagger and redoc settings to be overridden - #187
terencehonles wants to merge 1 commit into
Conversation
axnsan12
left a comment
axnsan12
left a comment
There was a problem hiding this comment.
The motivation seems reasonable, but seems to require a lot of big changes.
Could you point to some other examples of how libraries deal with this? For instance, how does DRF itself acoomodate multiple sites?
If I'm getting it right, the gist of it is that all global settings should have a class or object-level alternative of obtaining the same effect?
684db20 to
ff5d574
Compare
|
@axnsan12 I'll have to see how DRF handles this itself. We've not had to tweak anything on DRF since most of the global settings have worked ok for us. As far as the gist of it, that's correct. I'm just holding a reference to the user supplied settings object wherever it was needed. I aliased the global one to have a "_" prefix so linting would alert another developer that there was no non prefixed version of the settings in the local context and they should figure out if they should use the global context or if context was supplied locally. In the long term it would probably be best to remove the entry points for using the global settings so there's no confusion for future developers, but in order to maintain backward compatibility I kept the global settings as a fallback. If you want comments or me to put the global fallbacks into a helper function w/ a deprecation warning I can change the code to do that. |
|
Going through http://www.django-rest-framework.org/api-guide/settings/ most of the settings we would change on DRF would be possible through creating a base class per host and using that to supply overides to the permission classes or authentication classes. In our site we've not tried sharing serializers between sites so this hasn't been something we'd needed to worry about. DRF stays relatively extensible because it has an easy place to put the hooks to override things. |
|
Hello! I've been looking into it, and the approach of this PR doesn't seem optional to me. However the general idea is solid and reasonable, so I'll be working to decouple/phase out global settings and, as you say, increase the extensibility of the various components that are currently using global settings. But the approach I'm imagining would be more localized rather than hauling the whole settings objects around everywhere. |
|
I'm guessing you mean optimal rather than optional. Anyways, I'll be maintaining this PR (as we are using the changes) until the rearchitecturing is made. I wrote the PR this way to integrate as easy as possible into the existing codebase without imposing changes to the architecture. I look forward to seeing the changes you envision. |
ff5d574 to
3ed59d7
Compare
f8471c1 to
9288d4a
Compare
9288d4a to
2e8a4dd
Compare
2e8a4dd to
fc4523a
Compare
fc4523a to
917a20a
Compare
917a20a to
879906f
Compare
879906f to
21b7c16
Compare
21b7c16 to
1b6ad4c
Compare
JoelLefkowitz
added
1.21.x
d5f835d to
28fc25b
Compare
28fc25b to
92e3183
Compare
92e3183 to
ac0cddd
Compare
ac0cddd to
4e2c734
Compare
4e2c734 to
f7cba82
Compare
f7cba82 to
0987938
Compare
0987938 to
64d4042
Compare
64d4042 to
dc342e7
Compare
dc342e7 to
0e21eb1
Compare
0e21eb1 to
f751c04
Compare
f751c04 to
9f1f107
Compare
e03791d to
db3cc47
Compare
db3cc47 to
0a85191
Compare
When using site middleware there is no way to configure swagger instances to have different login URLs or authentication mechanisms. This change updates any global references to app settings to prefer a local setting if it exists, and for backward compatibility it refers to the global setting by default. The following calls will need to be adjusted to account for the new swagger_settings parameter that is passed: - ``drf_yasg.utils.get_produces(...)`` - ViewInspector class (and user defined subclasses) - Generator class (and user defined subclasses)
0a85191 to
0aab487
Compare
3 participants
When using site middleware there is no way to configure swagger instances to have different login URLs or authentication mechanisms. This change updates any global references to app settings to prefer a local setting if it exists, and for backward compatibility it refers to the global setting by default.
The following calls will need to be adjusted to account for the new swagger_settings parameter that is passed:
drf_yasg.utils.get_produces(...)