User tests: Successful: Unsuccessful:
Pull Request for comment #37592 (comment).
Removes Replaces the CMS/Filesystem dependencies in the classmap generator.
The removed file is regenerated and the site loads as expected.
The removed file is regenerated and the site loads as expected.
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
Labels |
Added:
?
|
I have tested this item
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
Please don't blindly replace File::write with file_put_contents thanks. There are multiple reasons to keep it as it is.
@HLeithner It seems there are different opinions because @wilsonge approved it. Am setting the RLDQ label to flag it for further discussion/decision.
@HLeithner please elaborate why this should not work. It was not a blind replacement. I'v even tested it when the cache folder doesn't exist, it loads the map into the ram thanks to #33263. The site crashes afterwards because the cachecontroller can't find the folder. So we are pretty safe here to move to native PHP.
Labels |
Added:
?
?
|
FIle::write does much more then just write a file to disk. it creates the folder (ok should exists) but most important it invalidates the opcache.
Never the less on the roadmap on j5 we have the point to use asynchronous functions where it make sense and where it's possible. Don't know yet if it's useful at this point but at least should be considered.
this is a great example of why low level changes need much better test instructions and descriptions
Status | Ready to Commit | ⇒ | Pending |
Back to pending due to reasons stated above. I should have remembered the opcache thing when setting RTC.
this is a great example of why low level changes need much better test instructions and descriptions
And this is a great example how to not participate in an open source project as it doesn't bring any value and pisses of contributors. Instead of propose a suggestion what needs to be changed
but most important it invalidates the opcache
My opcache settings are as follows, so what do I need to change to make the pr fail when opcache kicks in as it worked on my end?
My opcache settings are as follows, so what do I need to change to make the pr fail when opcache kicks in as it worked on my end?
@laoneo You could try with the settings mentioned in the description of PR #36878 :
Use a server with OPcache enabled and the settings
opcache.validate_timestamps=1
andopcache.revalidate_freq=300
Your screenshot doesn't show the values for these settings.
Labels |
Added:
?
Removed: ? |
Now I can reproduce the opcache issue. Reverted to use the framework package write function, so we don't need the full CSM stack.
So I guess we can do another round of testing as it does the same as before, just using the framework package.
Now the only remaining change is the alpha ordering of the use statements. I think this can be tested by review.
@laoneo Could you update the description of the PR accordingly? Thanks in advance.
Silly me ... was confused by the almost same use statements. But I see it changes from CMS to framework package.
Labels |
Removed:
?
?
|
The framework throws exceptions which was a return false before. this have to be caught. Both File::write and Folder:folders need to be in a try catch block now.
Catching now the exceptions.
Status | Pending | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-05-04 12:23:35 |
Closed_By | ⇒ | HLeithner |
thanks
@Fedik here you have a filesystem pr.