search

known-as-bmf / plugin.video.arteplussept

Plugin Kodi (ex XBMC) permettant de voir les vidéos disponibles sur Arte +7
GNU General Public License v2.0
44 stars 23 forks source link

use datetime with non en locales #74

Closed seckels closed 3 years ago

seckels commented 3 years ago

This should fix issue #64.

seckels commented 3 years ago

As far as I can tell this is not fixed in the code yet?

Thank you for the nice plugin, by the way.

known-as-bmf commented 3 years ago

Could you give an example on when this fix is needed ?

Specific endpoint, add-on language, kodi language, system language ?

seckels commented 3 years ago

datetime.strptime uses LC_TIME of the current locale to determine in which language the time is given. In the code the date_string is in english, so also LC_TIME should be set to english. The locale "C" is the default language setting, which is basically "en_US" and enabled on every system.

So the error of issue #64 occurs if the locale in kodi is set to a different value than "C", "en_US" or similar.

kodi tries to set its locale according to its language and region settings (under Settings->Interface->Regional). If the locale is enabled on the system, it should work and the log should include something like

2021-03-20 21:31:17.898 T:30069   DEBUG <general>: trying to set locale to fr_FR.UTF-8
2021-03-20 21:31:17.898 T:30069    INFO <general>: global locale set to fr_FR.UTF-8

If the requested locale is not enabled on the system, this fails and the default "C" locale is used.

The error should occur, as soon as parse_date is executed, for example when opening the "Most recent" category. This seems to be true, only for the plugin language settings of german and french. As far as I can see, the api does not give "broadcastBegin" entries in the other languages, so parse_date is not called and thus no errors occur.

And on android this error should not be triggered, because on android kodi always sets its locale to "C".

So, a lot of specifics to fulfill to get this error... I hope this helps for testing or do you need anything else?

known-as-bmf commented 3 years ago

Thank you for the thorough explanation.

I see some examples online that use a locking mechanism to prevent multi-threading issues. Do you think it is applicable here?

hypnotoad commented 3 years ago

Thank you for the thorough explanation.

I see some examples online that use a locking mechanism to prevent multi-threading issues. Do you think it is applicable here?

The problem is that all local-specific calls have to be protected everywhere in the kodi process. However, usually the UI is single-threaded (I assume that the parsing is done in the main thread), so setting and restoring might be safe.

Maybe a better alternative would be to use the babel library which was made for exactly this purpose: https://stackoverflow.com/questions/985505/locale-date-formatting-in-python

seckels commented 3 years ago

You're absolutely right. We can't be sure that there aren't any other threads. So it would be better to use a different library without the need to globally change the locale. babel is not in the kodi repository, but dateutil is.

I think this is a good solution. And it gets rid of the NoneType workaround.

What do you think?

hypnotoad commented 3 years ago

Looking good from my side!

known-as-bmf commented 3 years ago

Thanks to both of you for your contribution!

I think I will update the dependencies version to match

in a future commit.