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

Feature/487-Use-django-caching-to-improve-TTFB #646

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

LittleBigProgramming
Copy link
Contributor

What GitHub issue does this PR apply to?

Resolves #487

What changed and why?

Use database caching per site for an hour using timeout of 3600 on non development environments that will use Dummy Caching (https://docs.djangoproject.com/en/4.1/topics/cache/#dummy-caching-for-development)

Site-wide cache will be stored in cache_table, to run this initially the command of python manage.py createcachetable will need to be run after the deployment of this feature.

This uses the default django database caching settings with the schema of

CREATE TABLE IF NOT EXISTS "cache_table" (
    "cache_key" varchar(255) NOT NULL PRIMARY KEY,
    "value" text NOT NULL,
    "expires" datetime NOT NULL
);
CREATE INDEX "cache_table_expires" ON "cache_table" ("expires");

Please see the following screenshot of the cache
Screenshot 2022-10-27 at 21 08 46

Checklist

  • I claimed any associated issue(s) and they are not someone else's
  • I have looked at documentation to ensure I made any revisions correctly
  • I tested my changes locally to ensure they work
  • (If editing Django) I have added or edited any appropriate unit tests for my changes

Any additional comments or things to be aware of while reviewing?

@geekygirlsarah Please let me know if that issue persists with the branch being out of date. I synced and pulled from the upstream on my fork before making this PR so it should be okay 🤔

development environments which will use Dummy Caching
(https://docs.djangoproject.com/en/4.1/topics/cache/#dummy-caching-for-development)

Site wide cache will stored in cache_table, to run this initially the
command of `python manage.py createcachetable` will need to be run after
the deployment of this feature.
@geekygirlsarah geekygirlsarah temporarily deployed to codethesaurus-test-pr-646 October 29, 2022 15:00 Inactive
Copy link
Member

@geekygirlsarah geekygirlsarah left a comment

Choose a reason for hiding this comment

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

I think this can work, but probably needs the migration to make it happen.

I think you can run python manage.py makemigrations and it should give you a new file to commit to create the database table for the cache.

@LittleBigProgramming
Copy link
Contributor Author

I think this can work, but probably needs the migration to make it happen.

I think you can run python manage.py makemigrations and it should give you a new file to commit to create the database table for the cache.

I didn’t realize you could make the migrations like this, I will add it in when I get some time. Thanks! 🙏

@geekygirlsarah geekygirlsarah temporarily deployed to codethesaurus-test-pr-646 November 9, 2022 19:33 Inactive
@geekygirlsarah
Copy link
Member

I'm commenting that I see this update, but because this need more rigorous testing, I'll have to do it when I'm not out of town.

@geekygirlsarah geekygirlsarah temporarily deployed to codethesaurus-test-pr-646 November 15, 2022 23:05 Inactive
@LittleBigProgramming
Copy link
Contributor Author

Hi @geekygirlsarah ,

I've tried adding the migrations by running the following commands. Am I missing something here?

Screenshot 2022-11-15 at 23 42 31

I know the raw output to add the table is

CREATE TABLE IF NOT EXISTS "cache_table" (
    "cache_key" varchar(255) NOT NULL PRIMARY KEY,
    "value" text NOT NULL,
    "expires" datetime NOT NULL
);
CREATE INDEX "cache_table_expires" ON "cache_table" ("expires");

Thanks!

@geekygirlsarah
Copy link
Member

@LittleBigProgramming I'm revisiting some stuff around here and was looking at your PR, and I can't seem to update it to pull in the latest updates and test it. I'll try to do it manually on my machine but you'll have to do it on your repo/branch.

@LittleBigProgramming
Copy link
Contributor Author

@LittleBigProgramming I'm revisiting some stuff around here and was looking at your PR, and I can't seem to update it to pull in the latest updates and test it. I'll try to do it manually on my machine but you'll have to do it on your repo/branch.

Sorry I was on holiday, I can try and update the branch so you can test it!

@LittleBigProgramming
Copy link
Contributor Author

@LittleBigProgramming I'm revisiting some stuff around here and was looking at your PR, and I can't seem to update it to pull in the latest updates and test it. I'll try to do it manually on my machine but you'll have to do it on your repo/branch.

Sorry I was on holiday, I can try and update the branch so you can test it!

@geekygirlsarah I have updated al the Github action checks seemed to pass okay! 👀

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.

Use django caching to improve TTFB
2 participants