Fixing failed test#809
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #809 +/- ##
========================================
- Coverage 38.2% 38.1% -0.0%
========================================
Files 48 48
Lines 3727 3879 +152
Branches 301 335 +34
========================================
+ Hits 1420 1476 +56
- Misses 2006 2068 +62
- Partials 301 335 +34
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
for more information, see https://pre-commit.ci
…django-redis into fixing_failed_test
|
why so many new methods were added? |
| def patch_itersize_setting() -> Iterable[None]: | ||
| # destroy cache to force recreation with overriden settings | ||
| del caches["default"] | ||
| with suppress(AttributeError): |
There was a problem hiding this comment.
perhaps this should be solved somehow different? why is it needed? which Django version introduced this error?
| value = int(value) | ||
| except (ValueError, TypeError): | ||
| with suppress(ValueError, TypeError): | ||
| return float(value) |
There was a problem hiding this comment.
we do not have incrbyfloat, why add it to hash commands?
|
I would have added separately the changes, this is not just a fix |
I was trying to solve this : #766 |
ulgens
left a comment
ulgens
left a comment
There was a problem hiding this comment.
Not sure why my review was requested, but the PR seems to be handling multiple things context-wise. I wouldn't merge this without a clear focus first.
|
@MehrazRumman it looks like you merged the commits for your fix (gzip related stuff) with #806 , can you make sure everything is properly in #806 that should be in #806 and remove those changes from this branch. I suspect you may just want to cherry pick 190668e, 5e4ed9d, and 01c59bc and re-push that to this branch. It looks like you have some other code churn, but you should figure out if they are related or even intended, and if you want open another PR that just makes those changes. |
4 participants
see #808
Trying to fix the failed test in CI