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

CLDR-16900 Avoid 304 Not Modified; instead use boolean json.unchanged #3597

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

btangmu
Copy link
Member

@btangmu btangmu commented Apr 2, 2024

-Standard usage of 304 would involve adding headers and getting cached json from browser; this is more efficient

-Also restore and revise tools/cldr-apps/js/test/run.sh for convenience (CLDR-17481)

CLDR-16900

  • This PR completes the ticket.

ALLOW_MANY_COMMITS=true

@btangmu btangmu self-assigned this Apr 2, 2024
@btangmu btangmu requested review from srl295 and macchiati April 2, 2024 17:44
-Standard usage of 304 would involve adding headers and getting cached json from browser; this is more efficient

-Also restore and revise tools/cldr-apps/js/test/run.sh for convenience
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@macchiati
Copy link
Member

I looked this over but @srl295 should be the one to review.

@srl295
Copy link
Member

srl295 commented Apr 2, 2024

-Standard usage of 304 would involve adding headers and getting cached json from browser; this is more efficient

I don't follow this.. which headers would need to be added?

Approving so we can move it forward, just asking

@btangmu
Copy link
Member Author

btangmu commented Apr 3, 2024

I don't follow this.. which headers would need to be added?

@srl295 https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/304 says "This response code is sent when the request is a conditional GET or HEAD request with an [If-None-Match] or an [If-Modified-Since] header and the condition evaluates to false."

It seems that under those conditions, the browser (receiving 304) may deliver a cached response to the client application (which then receives 200) -- it's transparent to the client app, assuming the browser has the cached response. See https://www.hostinger.com/tutorials/304-status-code

I'm not sure but returning 304 when the request lacks such headers might be unconventional or worse...

@btangmu btangmu merged commit f1401ba into unicode-org:main Apr 3, 2024
11 checks passed
@btangmu btangmu deleted the t16900_e branch April 3, 2024 01:10
@srl295
Copy link
Member

srl295 commented Apr 3, 2024

I don't follow this.. which headers would need to be added?

@srl295 https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/304 says "This response code is sent when the request is a conditional GET or HEAD request with an [If-None-Match] or an [If-Modified-Since] header and the condition evaluates to false."

It seems that under those conditions, the browser (receiving 304) may deliver a cached response to the client application (which then receives 200) -- it's transparent to the client app, assuming the browser has the cached response. See https://www.hostinger.com/tutorials/304-status-code

I'm not sure but returning 304 when the request lacks such headers might be unconventional or worse...

that does make sense.

we could use the if-modified-since field and expiry times - and use a date instead of an announcement number, and the benefit is that the browser would do the caching for us, even between multiple windows querying the same server.. but you're right, it would bring other headaches.

Another possibility which I've used productively is http://http.cat/204 204 no content but that apparently is cacheable by default also.

@btangmu
Copy link
Member Author

btangmu commented Apr 3, 2024

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.

3 participants