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: issue when clearing cache with opcache with enable_file_override #33658

Merged
merged 1 commit into from Aug 26, 2023

Conversation

Tofandel
Copy link
Contributor

@Tofandel Tofandel commented Aug 18, 2023

Questions Answers
Branch? 8.1.x
Description? With op cache enabled with enable_file_override=1 and validate_timestamps=0 when clearing the cache from the admin the var/cache/prop/appParameters.php file is created with permissions 000 and is then unreadable and needs to be removed manually, this is because file_exists will return true, but fileperms will return 0 because the file doesn't actually exist
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
How to test? Enable opcache with the settings, go to clear the cache, if there is no persisting error 500 it's working correctly

In any case it's a good idea to also reset opcache when the user clears the cache manually

@Tofandel Tofandel requested a review from a team as a code owner August 18, 2023 16:53
@prestonBot
Copy link
Collaborator

Hi, thanks for this contribution!

I found some issues with the Pull Request description:

  • Your pull request does not seem to fix any issue, consider creating one (see note below) and linking it by writing Fixes #1234.

Would you mind having a look at it? This will help us understand how interesting your contribution is, thank you very much!

About linked issues

Please consider opening an issue before submitting a Pull Request:

  • If it's a bug fix, it helps maintainers verify that the bug is effectively due to a defect in the code, and that it hasn't been fixed already.
  • It can help trigger a discussion about the best implementation path before a single line of code is written.
  • It may lead the Core Product team to mark that issue as a priority, further attracting the maintainers' attention.

(Note: this is an automated message, but answering it will reach a real human)

@prestonBot prestonBot added 8.1.x Branch Bug fix Type: Bug fix labels Aug 18, 2023
@kpodemski
Copy link
Contributor

hey @Tofandel

there are some problems with your PR:

  1. There's problem with code formatting
  2. There's a merge commit available, you need to use rebase option instead of regular pull in order to sync your branch with the upstream branch.

You can also propose this change against the 8.1.x branch because it's a valid improvement 👍🏻

@kpodemski kpodemski added the Waiting for author Status: action required, waiting for author feedback label Aug 22, 2023
@Tofandel Tofandel changed the base branch from develop to 8.1.x August 22, 2023 10:30
@Tofandel
Copy link
Contributor Author

I rebased on 8.1.x and fixed the style issue

@prestashop-issue-bot prestashop-issue-bot bot removed the Waiting for author Status: action required, waiting for author feedback label Aug 22, 2023
@kpodemski kpodemski closed this Aug 23, 2023
@kpodemski kpodemski reopened this Aug 23, 2023
@ps-jarvis ps-jarvis added the Waiting for QA Status: action required, waiting for test feedback label Aug 23, 2023
@hibatallahAouadni hibatallahAouadni self-assigned this Aug 25, 2023
Copy link
Contributor

@hibatallahAouadni hibatallahAouadni left a comment

Choose a reason for hiding this comment

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

Hello @Tofandel

LGTM, QA ✅ and auto tests are 🟢 except the Product V2 and it's not related to your PR.
https://github.com/hibatallahAouadni/testing_pr/actions/runs/5975852106

Thanks!

@hibatallahAouadni hibatallahAouadni added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Aug 26, 2023
@prestonBot
Copy link
Collaborator

QA approved, well done! Message to the maintainers: do not forget to milestone it before the merge.

@FabienPapet FabienPapet added this to the 8.1.2 milestone Aug 26, 2023
@FabienPapet FabienPapet merged commit 0936c1f into PrestaShop:8.1.x Aug 26, 2023
56 checks passed
@jolelievre jolelievre changed the title fix: issue when clearing cache with opcache with enable_file_override Fix: issue when clearing cache with opcache with enable_file_override Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.1.x Branch Bug fix Type: Bug fix QA ✔️ Status: check done, code approved
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

8 participants