On 05/02/2012 09:49 AM, Vincent Massol wrote: > > On May 2, 2012, at 3:55 PM, Caleb James DeLisle wrote: > >> >> >> On 05/02/2012 09:20 AM, Vincent Massol wrote: >>> Hi Caleb, >>> >>> On May 2, 2012, at 3:12 PM, Caleb James DeLisle wrote: >>> >>>> Just committed the patch to a branch, you can see it here >>>> https://github.com/xwiki/xwiki-commons/commit/32fb77049fe9d8f758e776d26bfe5d7d2d21c4cc >>>> >>>> Since it contains an API break in EnvironmentConfiguration, I wanted to >>>> run it by the list before merging it into the master. >>>> I'll push later on today if nobody objects. >>> >>> We should no longer break APIs. So this will need to go through deprecation >>> and legacy. >>> >>> Why is File an issue? >>> >>> Big -1 to the code I've seen. We should never ever use System.exit(). >> >> How do you envision the wiki behaving as a whole if there is no writable >> temp dir? >> I agree that killing the jvm is very bad if there is any way to recover but >> this seems like the edge case where there is no way out. > > There's no reason to kill the JVM IMO. > > The only thing we should do is to not make the XWiki webapp available in the > running container. This is done by throwing a top level exception. > > Remember that a servlet container can be used to run several webapps, not > just xwiki. Killing it is really really bad.
On second thought I agree since all other "it's dead" errors such as database connection stuff results in an error page for the http request. Thanks for calling me out. Caleb > > Thanks > -Vincent > >>> Also I've seen a lot of "final" used in the code. Please don't use them >>> since they're unnecessary and bad in most cases and we don't have any best >>> practice to use them in methods for ex (this prevents extensibility). As >>> for variables I don't see the point at all. >> >> I can remove the final annotation on the methods, I think it's important >> however to differentiate between clean extensibility and using "extends" to >> monkey patch code in ways that weren't intended. >> As for variables and fields, I am very fond of making those final because >> they are inline documentation and verification. >> I declare my intent not to change a variable with the final keyword, the >> reader/maintainer knows what I meant and the compiler checks that I didn't >> make a mistake. >> If you want me to strip those, I can but think it's like stripping comments. >> >> Caleb >> >>> >>> Re the fail fast I was expecting some event listener listening on >>> Application started event to check if the directories were writable for >>> example and throw some exception if not. >>> >>> Thanks >>> -Vincent >>> >>>> Caleb >>>> >>>> >>>> On 04/23/2012 09:43 AM, Caleb James DeLisle wrote: >>>>> Hi, >>>>> >>>>> In order to fix XWIKI-7748 and XWIKI-7749, we need a plan for deleting >>>>> temporary file. >>>>> XWikiAttachmentContent uses File.deleteOnExit(), but in Sun's >>>>> implementation, deleteOnExit() >>>>> only deletes on a *clean* exit, in a crash it can't delete the files but >>>>> it doesn't tag them >>>>> so they can be deleted in the next run either. Since temp files are >>>>> usually pseudo-randomly >>>>> named, they are impossible to find and delete later. >>>>> >>>>> I propose that we create a subdirectory, `java.io.tmpdir`/xwiki/ which >>>>> will be removed on >>>>> application exit. If the jvm crashes, the files will be removed next time >>>>> around. Then alter >>>>> ApplicationContext.getTemporaryDirectory() to yield this directory. >>>>> >>>>> WDYT? >>>>> >>>>> Caleb >>> _______________________________________________ >>> devs mailing list >>> [email protected] >>> http://lists.xwiki.org/mailman/listinfo/devs >>> >> >> _______________________________________________ >> devs mailing list >> [email protected] >> http://lists.xwiki.org/mailman/listinfo/devs > > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs > _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
