doc: Adds updated documentation on middleware ordering, issue #302 #758
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #758 +/- ##
==========================================
- Coverage 87.21% 87.10% -0.11%
==========================================
Files 52 52
Lines 2135 2141 +6
==========================================
+ Hits 1862 1865 +3
- Misses 273 276 +3 ☔ View full report in Codecov by Sentry. |
albertyw
reviewed
Dec 1, 2024
README.md
Outdated
| @@ -56,7 +56,7 @@ INSTALLED_APPS = ( | |||
| ) | |||
| ``` | |||
|
|
|||
| **Note:** The middleware placement is sensitive. If the middleware before `silk.middleware.SilkyMiddleware` returns from `process_request` then `SilkyMiddleware` will never get the chance to execute. Therefore you must ensure that any middleware placed before never returns anything from `process_request`. See the [django docs](https://docs.djangoproject.com/en/dev/topics/http/middleware/#process-request) for more information on this. | |||
| **Note:** The order of middleware is sensitive. If any middleware placed before `silk.middleware.SilkyMiddleware` returns a response without invoking its `get_response`, the `SilkyMiddleware` won’t run. To avoid this, ensure that middleware preceding `SilkyMiddleware` does not bypass or return a response without calling its `get_response`. For further details, check out the [Django documentation](https://docs.djangoproject.com/en/5.1/topics/http/middleware/#middleware-order-and-layering). | |||
There was a problem hiding this comment.
Let's continue to use dev because django-silk supports multiple versions of Django and so that this link doesn't need to be updated every time there's a new django release.
There was a problem hiding this comment.
I applied the changes through the commits.
docs/troubleshooting.rst
Outdated
|
|
||
| This `GitHub issue <https://github.com/jazzband/django-silk/issues/12>`_ also has information on dealing with middleware problems. | ||
| This `GitHub issue <https://github.com/jazzband/django-silk/issues/12>`_ also has information on dealing with middleware problems.` |
There was a problem hiding this comment.
This is a very old issue and doesn't seem to give much additional context than the above text.
There was a problem hiding this comment.
I applied the changes through the commits.
Changing the link to the developer documentation
Removing old issue link
|
I applied the changes through the commits. |
albertyw
reviewed
Dec 2, 2024
docs/troubleshooting.rst
Outdated
| The middleware is placement sensitive. If the middleware before ``silk.middleware.SilkyMiddleware`` returns from ``process_request`` then ``SilkyMiddleware`` will never get the chance to execute. Therefore you must ensure that any middleware placed before never returns anything from ``process_request``. See the `django docs <https://docs.djangoproject.com/en/dev/topics/http/middleware/#process-request>`_ for more information on this. | ||
|
|
||
| This `GitHub issue <https://github.com/jazzband/django-silk/issues/12>`_ also has information on dealing with middleware problems. | ||
| The order of middleware is sensitive. If any middleware placed before ``silk.middleware.SilkyMiddleware`` returns a response without invoking its ``get_response``, the ``SilkyMiddleware`` won’t run. To avoid this, ensure that middleware preceding ``SilkyMiddleware`` does not bypass or return a response without calling its ``get_response``. For further details, check out the `Django documentation <https://docs.djangoproject.com/en/5.1/topics/http/middleware/#middleware-order-and-layering>`. |
There was a problem hiding this comment.
Let's also keep this link on the django dev version too
There was a problem hiding this comment.
Ok. I added the changes on the new commit.
Changing the documentation link to the developer version
albertyw
approved these changes
Dec 2, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #302