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: remove PHP cache validation disabling example #12079

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joshtrichards
Copy link
Member

☑️ Resolves

The way this is presented it seems people skim and enter this example blindly without reading the surrounding text.

We don't want people thinking we recommend this for all environments.

If one wishes to disable validation, that's fine (as noted in the text), but they presumably know what they're doing and can figure it out from the PHP manual on their own.

I think @kesselb hinted about removing this part during the last edit round of this area. ;-)

🖼️ Screenshots

Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards
Copy link
Member Author

/backport to stable29

@joshtrichards
Copy link
Member Author

/backport to stable28

@kesselb
Copy link
Contributor

kesselb commented Jul 31, 2024

Any Server/app upgrades or changes to config.php will then require restarting PHP (or otherwise manually clearing the cache or invalidating this particular script).

Should we remove the quoted part as well?

I think @kesselb hinted about removing this part during the last edit round of this area. ;-)

Yep. As you wrote, people just copy it because it's on the server tuning page and have no idea about the impact.

@MichaIng
Copy link
Member

MichaIng commented Jul 31, 2024

Hmm, do we really want to remove valuable information just because some people do not do what docs are made for: reading?

I personally actually do recommend disabling PHP revalidation for serious production Nextcloud systems where you want to max out performance = user experience was much as possible. I personally have a little command which clears config.php from OPcache via opcache-gui web API, for cases where I do not want to restart PHP. When upgrading Nextcloud from console, I restart PHP anyway, as the downtime is there already.

No problem with marking warnings clearer, putting the whole info into a warning box, whatever, but we are not Apple here (sorry for the bias/cliche 😉) where we offer just one failsafe default to prevent users (and we are talking about "admins" here) from configuring things wrong. People do things wrong, and they can learn from this, especially as admin valuable/essential.

Just my two cents 😉.

Should we remove the quoted part as well?

Makes sense then, as this absolutely belongs to disabling revalidation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants