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

allow swagger and redoc settings to be overridden #187

Open
wants to merge 1 commit into
base: 1.21.x
Choose a base branch
from

Conversation

terencehonles
Copy link
Contributor

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)

@axnsan12 axnsan12 self-requested a review September 9, 2018 22:03
Copy link
Owner

@axnsan12 axnsan12 left a comment

Choose a reason for hiding this comment

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

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?

@terencehonles terencehonles force-pushed the all-swagger-and-redoc-settings-to-be-overridden branch from 684db20 to ff5d574 Compare September 15, 2018 19:16
@terencehonles
Copy link
Contributor Author

@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.

@terencehonles
Copy link
Contributor Author

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.

@axnsan12
Copy link
Owner

axnsan12 commented Oct 9, 2018

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.

@terencehonles
Copy link
Contributor Author

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.

@terencehonles terencehonles force-pushed the all-swagger-and-redoc-settings-to-be-overridden branch from ff5d574 to 3ed59d7 Compare October 26, 2018 19:09
@terencehonles terencehonles force-pushed the all-swagger-and-redoc-settings-to-be-overridden branch 3 times, most recently from f8471c1 to 9288d4a Compare May 11, 2019 00:35
@terencehonles terencehonles force-pushed the all-swagger-and-redoc-settings-to-be-overridden branch from 9288d4a to 2e8a4dd Compare August 21, 2019 22:56
@terencehonles terencehonles force-pushed the all-swagger-and-redoc-settings-to-be-overridden branch from 2e8a4dd to fc4523a Compare December 4, 2019 00:29
@terencehonles terencehonles force-pushed the all-swagger-and-redoc-settings-to-be-overridden branch from fc4523a to 917a20a Compare February 24, 2020 18:04
@terencehonles terencehonles force-pushed the all-swagger-and-redoc-settings-to-be-overridden branch from 917a20a to 879906f Compare October 26, 2020 20:13
@terencehonles terencehonles force-pushed the all-swagger-and-redoc-settings-to-be-overridden branch from 879906f to 21b7c16 Compare November 5, 2020 03:35
@terencehonles terencehonles force-pushed the all-swagger-and-redoc-settings-to-be-overridden branch from 21b7c16 to 1b6ad4c Compare September 24, 2021 01:57
@JoelLefkowitz JoelLefkowitz changed the base branch from master to 1.21.x July 17, 2022 17:20
@JoelLefkowitz JoelLefkowitz added 1.21.x Release target in 1.21.x feature New feature proposal 1.22.x Release target in 1.22.x 1.23.x Release target in 1.23.x and removed 1.21.x Release target in 1.21.x 1.22.x Release target in 1.22.x labels Jul 17, 2022
@terencehonles terencehonles force-pushed the all-swagger-and-redoc-settings-to-be-overridden branch 2 times, most recently from d5f835d to 28fc25b Compare August 11, 2022 10:27
@terencehonles terencehonles force-pushed the all-swagger-and-redoc-settings-to-be-overridden branch from 28fc25b to 92e3183 Compare November 1, 2022 09:42
@terencehonles terencehonles force-pushed the all-swagger-and-redoc-settings-to-be-overridden branch from 92e3183 to ac0cddd Compare March 2, 2023 16:11
@terencehonles terencehonles force-pushed the all-swagger-and-redoc-settings-to-be-overridden branch from ac0cddd to 4e2c734 Compare July 6, 2023 18:30
@terencehonles terencehonles force-pushed the all-swagger-and-redoc-settings-to-be-overridden branch from 4e2c734 to f7cba82 Compare September 7, 2023 09:27
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)
@terencehonles terencehonles force-pushed the all-swagger-and-redoc-settings-to-be-overridden branch from f7cba82 to 0987938 Compare September 7, 2023 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.23.x Release target in 1.23.x feature New feature proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants