Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix exception on iOS in _get_localzone #895

Closed
wants to merge 1 commit into from
Closed

Fix exception on iOS in _get_localzone #895

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 15 additions & 11 deletions babel/localtime/_unix.py
View file
Open in desktop
Expand Up @@ -65,17 +65,21 @@ def _get_localzone(_root='/'):
# since it knows where the zone files are that should be a bit
# better than reimplementing the logic here.
if sys.platform == 'darwin':
c = subprocess.Popen(['systemsetup', '-gettimezone'],
stdout=subprocess.PIPE)
sys_result = c.communicate()[0]
c.wait()
tz_match = _systemconfig_tz.search(sys_result)
if tz_match is not None:
zone_name = tz_match.group(1)
try:
return pytz.timezone(zone_name)
except pytz.UnknownTimeZoneError:
pass
try:
c = subprocess.Popen(['systemsetup', '-gettimezone'],
stdout=subprocess.PIPE)
sys_result = c.communicate()[0]
c.wait()
tz_match = _systemconfig_tz.search(sys_result)
if tz_match is not None:
zone_name = tz_match.group(1)
try:
return pytz.timezone(zone_name)
except pytz.UnknownTimeZoneError:
pass
# iOS doesn't come with systemsetup
except FileNotFoundError:
pass
Comment on lines +68 to +82
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having a huge try-block, you can make it more specific moving the non-critical things to the else-block of a try-except:

Suggested change
try:
c = subprocess.Popen(['systemsetup', '-gettimezone'],
stdout=subprocess.PIPE)
sys_result = c.communicate()[0]
c.wait()
tz_match = _systemconfig_tz.search(sys_result)
if tz_match is not None:
zone_name = tz_match.group(1)
try:
return pytz.timezone(zone_name)
except pytz.UnknownTimeZoneError:
pass
# iOS doesn't come with systemsetup
except FileNotFoundError:
pass
try:
c = subprocess.Popen(['systemsetup', '-gettimezone'],
stdout=subprocess.PIPE)
# iOS doesn't come with systemsetup
except FileNotFoundError:
pass
else:
sys_result = c.communicate()[0]
c.wait()
tz_match = _systemconfig_tz.search(sys_result)
if tz_match is not None:
zone_name = tz_match.group(1)
try:
return pytz.timezone(zone_name)
except pytz.UnknownTimeZoneError:
pass

it helps identifying which part we expect to throw the exception, and avoids catching any unexpected instances of this exceptions – which probably should stay unhandled so that the bug becomes visible and one gets a nice backtrace.

(not a maintainer, just randomly browsed through PRs)


# Now look for distribution specific configuration files
# that contain the timezone name.
Expand Down