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.

Sign up for GitHub

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

Jump to bottom

Disable storage #1330

Merged
merged 19 commits into from
Oct 11, 2023
Merged

Disable storage #1330

merged 19 commits into from
Oct 11, 2023

Conversation

brightbyte
Copy link
Contributor

Allow storage of parsoid output to be disabled based on the domain parameter
@brightbyte
Allow storage to be disabled by domain.
4211c5f 
Change-Id: Id1de27cb2be0222e3fb90c9b81ee102f84176d5d
@brightbyte
Fix default options template
dabaa17 
Change-Id: Ia035a8a15840f097aa71236a80ec8c84d941fa9e
@brightbyte
Fix error handling in _getPageBundleFromParsoid
6924d02 
Change-Id: I276202dae0bb000fd1e7ea6962650c9310f1db5e
@brightbyte
Make linter happy
c165873 
Change-Id: I4478b238908421c798c0515ee576f7897006df59
@brightbyte
Include revision ID in generic etag
7203b12 
Also: fix the expected content of
https://de.wikipedia.beta.wmflabs.org/wiki/RESTBase_Testing_Page.
Someone edited it...

Change-Id: I1aceb0f62c4095a2221707e43e38217babdde6ad
@brightbyte
Use disabled_storage setting to disable storage.
6dce2ad 
Several endpoints already have a way to disable storage. Use the same
machnism for parsoid. There is no need to use a regular expression to
match domains, because the config can be defined for each domain
separately.

Note that the disabled_storage flag is set for some test and beta domains
in production, which would cause parsoid storage to be disabled on these
domains as well when this change is deployed. See https://github.com/search?q=repo%3Awikimedia%2Fmediawiki-services-restbase-deploy%20wikipedia.org_no_storage&type=code

Change-Id: I02fcb262ab303113e2d1282a9d87bffabd6e4cf2
@brightbyte
Use camelCase, fix rebase
081c3ad 
Change-Id: Ib64bd72d3a3a7d089d752985714e57fe171f1551
@brightbyte
Update config.fullstack.test.yaml
e189866 
revert changes to the test config that don't belong here
@brightbyte
Fix call to _getContentWithFallback
086d631 
Change-Id: I78dcaacaff94af3390c1e0cd4ae86179e89927a6
@brightbyte
Fix expected text of test page.
d98b9cf 
https: //de.wikipedia.beta.wmflabs.org/w/index.php?title=RESTBase_Testing_Page
Change-Id: I2ad2ee21c0af90e575d8ae40aea4d71dce9af0b0
@brightbyte
Allow disabled_storage to be configured per domain
6a8e933 
Change-Id: I31f64be4d561f622e3bb43b223d9d08f32787c56
johngian
johngian requested changes Sep 21, 2023
View reviewed changes
projects/sys/default.wmf.yaml Show resolved Hide resolved
sys/parsoid.js Show resolved Hide resolved
sys/parsoid.js Outdated Show resolved Hide resolved
@johngian
Copy link
Collaborator

johngian commented Sep 21, 2023 •
edited
Loading

It might also be useful to add a header for the fact that parsoid storage is disabled on responses and add a test to assert that for a test wiki from the local env setup.
@johngian
Copy link
Collaborator

Do we have a phab ticket for reference?
@brightbyte
Copy link
Contributor Author

Do we have a phab ticket for reference?

https://phabricator.wikimedia.org/T344945
@brightbyte
Copy link
Contributor Author

It might also be useful to add a header for the fact that parsoid storage is disabled on responses and add a test to assert that for a test wiki from the local env setup.

Something like x-restbase-cache-disabled?
@brightbyte
Inject TID into HTML
19716c1 
Also adds x-restbase-cache headers.

Change-Id: If9a5f0298bce577de7bb9827326463e6f6ad37c6
@johngian
Copy link
Collaborator

It might also be useful to add a header for the fact that parsoid storage is disabled on responses and add a test to assert that for a test wiki from the local env setup.

Something like x-restbase-cache-disabled?

Yeah, so we know what kind of requests we are getting (in case we need to debug from the client side) and also to be able to test this.
@brightbyte
Copy link
Contributor Author

Something like x-restbase-cache-disabled?

Yeah, so we know what kind of requests we are getting (in case we need to debug from the client side) and also to be able to test this.

I implemented it in the latest version.
@brightbyte
Add tests that check x-restbase-cache headers
70a3e2c 
NOTE: these tests are currently failing. One is expected to fail, the
other should be passing. Looks like the headers are not actually getting
send, for some reason.

Change-Id: I395700ea27660d20aa76d790840f00769d592bd7
@brightbyte
Set x-restbase-cache header consistently
b3cc0ee 
NOTE: tests need refinement, we need a way to read the config and a
mechanism for purging the cache to provoke a cache miss.

Change-Id: I40d0765dae47c78dcbb83a16ae3952b771e59255
@brightbyte
Make disabled_storage configurable in tests
050166d 
Change-Id: I55a49ff4c8f23fa0225396779ffb70a878609045
@brightbyte
Fix nocache case
f0a0aad 
Change-Id: I1ddabd1cc1789574d64a17c2efa8687cd7ecd7aa
@brightbyte
Parsoid host: don't use HTTPS in CI
0cfdf8a 
Change-Id: Icb749f684c1982dc1293c97e6082dc5b870e1ede
@brightbyte
Skip rerender tests if storage is disabled
9ddae9d 
Change-Id: Ic831890ef303d970b3e07c6d822ff7f583cd9836
@brightbyte
Copy link
Contributor Author

For the record, I don't understand why the "should render & re-render independent revisions" test is now failing with storage enabled. The behavior for that case shouldn't have changed at all. Did I actually break something, or is it an unrelated issue?...
johngian
johngian approved these changes Oct 11, 2023
View reviewed changes
config.fullstack.test.yaml Outdated Show resolved Hide resolved
@johngian johngian merged commit 5a436df into wikimedia:master Oct 11, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants
@brightbyte @johngian