#794 Better caching

Open
elegaanz wants to merge 2 commits from better-caching into master
elegaanz commented 5 months ago (Migrated from github.com)

Based on @trinity-1686a’s patch.

I’m not sure it is a fix for #786 but it is better than nothing.

Based on @trinity-1686a's patch. I'm not sure it is a fix for #786 but it is better than nothing.
elegaanz commented 5 months ago (Migrated from github.com)
Owner

OK, my git history is broken, I need to fix that.

OK, my git history is broken, I need to fix that.
igalic (Migrated from github.com) reviewed 5 months ago
igalic (Migrated from github.com) left a comment

🙋

@@ -67,1 +67,4 @@
// NGINX (and maybe other software) sometimes sends ETags with a
// "W/" prefix, that we ignore here
.any(|s| s[1..s.len() - 1] == etag || s[3..s.len() - 1] == etag)
{
Plume_migration_agent commented 5 months ago

can one of you two explain what this does?

can one of you two explain what this does?
igalic commented 5 months ago (Migrated from github.com)
Owner

don’t we still have to to change the subscribe/follow/blah button from a form… to… well… a button?

don't we still have to to change the subscribe/follow/blah button from a form… to… well… a button?
trinity-1686a reviewed 5 months ago
@@ -67,1 +67,4 @@
// NGINX (and maybe other software) sometimes sends ETags with a
// "W/" prefix, that we ignore here
.any(|s| s[1..s.len() - 1] == etag || s[3..s.len() - 1] == etag)
{
trinity-1686a commented 5 months ago

this should probably get it’s comment actually.
We send Etags as 'some_tag', the old match does not verify the ', but check that whatever in-between is the right tag. Nginx apparently change the Etag to W/'some_tag', W/ indicating it’s a weak Etag, the content may not be byte-equal, but is semantically identical (which is imo a shitty but correct interpretation of the RFC, as Nginx send compressed data)

The new match ignore that W/ saying it’s a weak etag and so match if Nginx modified the header we sent previously

Comment might looks like : // if a strong or weak etag matches

this should probably get it's comment actually. We send Etags as `'some_tag'`, the old match does not verify the `'`, but check that whatever in-between is the right tag. Nginx apparently change the Etag to `W/'some_tag'`, W/ indicating it's a weak Etag, the content may not be byte-equal, but is semantically identical (which is imo a shitty but correct interpretation of the RFC, as Nginx send compressed data) The new match ignore that W/ saying it's a weak etag and so match if Nginx modified the header we sent previously Comment might looks like : // if a strong or weak etag matches
trinity-1686a commented 5 months ago
Owner

well there is that too, but these buttons are not here on the front page or a blog or user page. Anyway for post a better caching solution that does not require to query the database should be added

well there is that too, but these buttons are not here on the front page or a blog or user page. Anyway for post a better caching solution that does not require to query the database should be added
codecov[bot] commented 5 months ago (Migrated from github.com)
Owner

Codecov Report

Merging #794 into master will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #794      +/-   ##
==========================================
- Coverage   39.09%   39.07%   -0.03%     
==========================================
  Files          73       73              
  Lines        9756     9756              
  Branches     2233     2233              
==========================================
- Hits         3814     3812       -2     
- Misses       4885     4886       +1     
- Partials     1057     1058       +1     
# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/794?src=pr&el=h1) Report > Merging [#794](https://codecov.io/gh/Plume-org/Plume/pull/794?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/28576c1fa3ce5f3259952faa8af1ca1647aedaa5&el=desc) will **decrease** coverage by `0.02%`. > The diff coverage is `0.00%`. ```diff @@ Coverage Diff @@ ## master #794 +/- ## ========================================== - Coverage 39.09% 39.07% -0.03% ========================================== Files 73 73 Lines 9756 9756 Branches 2233 2233 ========================================== - Hits 3814 3812 -2 - Misses 4885 4886 +1 - Partials 1057 1058 +1 ```
This pull request can be merged automatically.
You are not authorized to merge this pull request.
This branch is out-of-date with the base branch
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.