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

Run django-upgrade manually #640

Merged
merged 1 commit into from Jan 12, 2023
Merged

Conversation

albertyw
Copy link
Member

@albertyw albertyw commented Jan 7, 2023

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
Copy link

codecov bot commented Jan 7, 2023

Codecov Report

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

❗ 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           

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

@SebCorbin
Copy link
Contributor

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

@albertyw albertyw merged commit 602921e into jazzband:master Jan 12, 2023
@albertyw albertyw deleted the django-upgrade branch January 12, 2023 03:27
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.

None yet

2 participants