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

Fix #638 Lazy access to Django settings. #639

Merged
merged 24 commits into from
Jun 12, 2023
Merged

Fix #638 Lazy access to Django settings. #639

merged 24 commits into from
Jun 12, 2023

Conversation

iurisilvio
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Jan 6, 2023

Codecov Report

Merging #639 (e7ee981) into master (65a4349) will increase coverage by 2.6%.
The diff coverage is 81.0%.

@@           Coverage Diff            @@
##           master    #639     +/-   ##
========================================
+ Coverage    57.5%   60.0%   +2.6%     
========================================
  Files          39      39             
  Lines        2528    2534      +6     
  Branches       74      69      -5     
========================================
+ Hits         1452    1520     +68     
+ Misses       1059    1001     -58     
+ Partials       17      13      -4     
Flag Coverage Δ
mypy 34.1% <38.1%> (+0.3%) ⬆️
tests 89.7% <100.0%> (+6.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
tests/test_backend.py 15.0% <60.0%> (+0.7%) ⬆️
django_redis/cache.py 97.8% <100.0%> (+0.1%) ⬆️
django_redis/client/herd.py 82.3% <100.0%> (+58.9%) ⬆️
tests/settings/sqlite_herd.py 100.0% <100.0%> (ø)

... and 1 file with indirect coverage changes

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

@iurisilvio
Copy link
Contributor Author

iurisilvio commented Jan 7, 2023

Broken tests are related to another issue #645.

@iurisilvio
Copy link
Contributor Author

HerdClient has no tests, I'm not happy about it, I'll try to write some basic tests at least.

@iurisilvio
Copy link
Contributor Author

I discovered even herd test settings existed, but were not running in tox, probably because tests didn't passed. It is working now. PR ready for review.

Copy link
Member

@WisdomPill WisdomPill left a comment

Choose a reason for hiding this comment

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

A few comments

django_redis/cache.py Outdated Show resolved Hide resolved
setup.cfg Show resolved Hide resolved
changelog.d/638.removal Outdated Show resolved Hide resolved
tests/test_backend.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
@iurisilvio iurisilvio requested a review from WisdomPill June 11, 2023 19:47
@iurisilvio
Copy link
Contributor Author

I agreed and did all requested changes, ready for a new review.

herd.CACHE_HERD_TIMEOUT = 2

@pytest.fixture
def patch_itersize_setting() -> Iterable[None]:
Copy link
Member

Choose a reason for hiding this comment

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

very weird one but I trust you :)

@WisdomPill WisdomPill merged commit 7faa82b into jazzband:master Jun 12, 2023
@foarsitter foarsitter mentioned this pull request Jun 16, 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