Hi, I have a lot of test errors and already fixed quite a lot, but haven't committed them yet.
Short version: how about ... instead of using Locale.ROOT and Timezone UTC, the initial value of the ThreadLocal return Locale.getDefault() and Timezone.getDefault() and can be set if needed ... so the handling is similar to older versions? Long version: When I've started to fix the forbidden api errors, I've only touched places, where I was quite sure that Locale.ROOT won't do any harm. But of course now we also have to face the other issues... The reason for having a thread local is ...: - that having the timezone/locale as an additional parameter would lead to a lot of methods to be marked deprecated (at least I would do it...) - I've read about use-cases where workbooks from different time zones need to be processed, so it's not only the current environment time zone - "A ThreadLocal can lead to hard-to-find errors ...": I think the hardest part is, making the new handling public. Another problem I see with thread locals is, the reuse of servlet threads. So the usage pattern would need to restore the original setting in a finally block. - "... in fact almost anybody would now see UTC-formatted dates instead of local timezone, or?" I guess so too, and the tests fail when my CET-timezone is ignored (the DE-locale is only affecting very few cases), i.e. I have to set it explicitly - "The java default is to either use the system default ..." As noted above, I would like to provide a multi timezone/locale approach, but still keep a lot of the poi api as-is. I guess the discussion will continue for a bit, so I create a patch file instead of committing the changes and check your feedback then. If you commit changes in the meantime, I'll update my version accordingly, so don't be blocked ;) Andi On 03.09.2015 16:25, Uwe Schindler wrote: > Hi, > > This may be only slightly related: to work around the forbidden-apis issue, > you can still pass Timezone.getDefault() to the Java APIs - this is not > forbidden. Forbidden APIs just wants to make sure you know what you are > doing. By writing Timezone.getDefault() you explicitely say what you are > doing. And maybe add a comment why you do this. The same applies for Locales > or Charsets. E.g., At some places in Lucene we explicitely use the charset of > the console, so we use Charset.getDefault() at all those places. > > Uwe > > ----- > Uwe Schindler > H.-H.-Meier-Allee 63, D-28213 Bremen > http://www.thetaphi.de > eMail: [email protected] > > >> -----Original Message----- >> From: Dominik Stadler [mailto:[email protected]] >> Sent: Thursday, September 03, 2015 4:05 PM >> To: POI Developers List >> Subject: Re: Time Zone Setting >> >> Hi, >> >> Sorry, but I fear that through this we change the default behavior of POI: >> * A ThreadLocal can lead to hard-to-find errors when people use POI in a >> multi-threading environment. While it is better in some cases than having a >> global static, it now requires the developer to ensure that the timezone is >> set >> for each thread that is started without a way to set it globally. >> * I think previously POI used the java default settings which look at the >> operating system, this is now broken as we have "UTC" as default, which >> potentially leads to many people seeing different results compared to >> before, in fact almost anybody would now see UTC-formatted dates instead >> of local timezone, or? >> * The java default is to either use the system default timezone or if it >> should >> be different then use -Duser.timezone when invoking the JRE, see e.g. >> http://stackoverflow.com/questions/2493749/how-to-set-a-jvm-timezone- >> properly >> >> So my approach would be to: >> * Not use the DateUtil.setTimezone() approach as it has quite some potential >> for regressions but also not use TimeZone.setDefault() in unit tests at all >> * Ensure that we set -Duser.timezone to a fixed value when running unit >> tests in the various CI systems. We already do this for user.language and >> user.country, so setting the timezone as well here seems a natural choice >> >> This way we do not change default behavior for users and still make the CI >> runs a bit more reproducible time-wise... >> >> Dominik. >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
