jazzband / django-silk

Silky smooth profiling for Django
MIT License
4.35k stars 333 forks source link

Run django-upgrade manually #640

Closed albertyw closed 1 year ago

albertyw commented 1 year ago

Debugging the failures in #620 , I ran django-upgrade manually to find what it was doing to files to cause the tests to fail. This PR contains the changes from running django-upgrade manually (and matches #620) except for the below change from django-upgrade which breaks tests and therefore is not included in this PR:

diff --git a/silk/model_factory.py b/silk/model_factory.py
index ba21fed..9a091c0 100644
--- a/silk/model_factory.py
+++ b/silk/model_factory.py
@@ -74,7 +74,7 @@ class RequestModelFactory:
         self.request = request

     def content_type(self):
-        content_type = self.request.META.get('CONTENT_TYPE', '')
+        content_type = self.request.headers.get('content-type', '')
         return _parse_content_type(content_type)

     def encoded_headers(self):

After this PR is landed, pre-commit CI can regenerate #620 and the auto-linter can continue to work. https://github.com/albertyw/django-silk/blob/master/.pre-commit-config.yaml#L54-L58 may need to be commented out though to prevent the above change from reappearing.

codecov[bot] commented 1 year ago

Codecov Report

Merging #640 (4d9d6c6) into master (deb9b1f) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 4d9d6c6 differs from pull request most recent head 4cf7ee8. Consider uploading reports for the commit 4cf7ee8 to get more accurate results

@@           Coverage Diff           @@
##           master     #640   +/-   ##
=======================================
  Coverage   86.33%   86.33%           
=======================================
  Files          52       52           
  Lines        2093     2093           
=======================================
  Hits         1807     1807           
  Misses        286      286           

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

SebCorbin commented 1 year ago

I'm ok with that but we could also fix tests and adopt request.headers (instead of request.META) to be more future-proof