2 May
2012
2 May
'12
5:39 p.m.
Lets try this again...
Replaced the poorly thought out exit() call with a throw of an unchecked exception.
The unchecked exception which I usually use to signal that the storage is broken due
to misconfiguration or malfunction is the UnexpectedException.
In order to use this in commons, I created commons-store, which now only contains that
one exception but in the future might contain other classes which are battle tested,
have already gone through their refactorings, and have few or no dependencies.
https://github.com/xwiki/xwiki-commons/commit/26c714ce33a78fe02546bb1137f4f…
WDYT?
Caleb
On 05/02/2012 10:09 AM, Caleb James DeLisle wrote:
On 05/02/2012 09:49 AM, Vincent Massol wrote:
On May 2, 2012, at 3:55 PM, Caleb James DeLisle wrote:
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
On 05/02/2012 09:20 AM, Vincent Massol wrote:
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. 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/32fb77049fe9d8f758e776d26bfe5…
>
> 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.
Thanks
-Vincent
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
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
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs