Skip to content

Conversation

@jaap3
Copy link
Contributor

@jaap3 jaap3 commented Oct 4, 2023

Simplify the configuration a bit by decoupling the defaults from Django settings lookup / legacy setting handling.

@jaap3 jaap3 marked this pull request as ready for review October 4, 2023 19:50
Copy link
Owner

@adamchainz adamchainz left a comment

Choose a reason for hiding this comment

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

No thanks. This isn’t simpler as it requires understanding data classes and __getattribute__, which is famously hard to implement correctly (I don’t think it is correct here).

@adamchainz adamchainz closed this Oct 5, 2023
@jaap3
Copy link
Contributor Author

jaap3 commented Oct 5, 2023

@adamchainz too bad, I probably should've clarified my motivations. I'm looking into some way to apply CORS settings to specific views (via decorator, or other ways), with the possibility of differing settings per view. One of the things that's making that harder is the global nature of the configuration. This was an initial attempt of making it possible to configure things in "other ways" (i.e. by passing it as an argument somewhere).

Would be nice to know what you think is incorrect about the __getattribute__ implementation.

@jaap3 jaap3 mentioned this pull request Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants