-
Notifications
You must be signed in to change notification settings - Fork 556
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.
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
Set session timezone to local timezone on startup #1482
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: maximsmol <[email protected]>
Signed-off-by: maximsmol <[email protected]>
Signed-off-by: maximsmol <[email protected]>
Signed-off-by: maximsmol <[email protected]>
Signed-off-by: maximsmol <[email protected]>
Thanks for your contribution! :) I am not familiar with the For example, suppose that the server is on timezone "tz0", user1 on tz1, user2 on tz2, user3 on tz3. Currently, all users calling
We end up with 3 different values. They represent the same time, only expressed under 3 different timezones, but still these are different values. Also, what concerns me is that what you get in pgcli is different from what you get on psql. That in itself seems problematic to me. At least, problematic enough that this feature should not be activated by default. Could you please comment on that and clarify the other changes in behaviour (if any)? I believe that this kind of "customization" should rather take place in "startup commands". The idea of such a feature has been expressed some time ago (see #247). What do you think? @j-bennet : what's your take on this? |
Thanks for responding so quickly. ConcernsTo summarize, there are two concerns:
In response:
The only other potential issue I can come up with is for people using Startup CommandsI think "startup commands" would be nice in addition to this PR. It's not a good replacement because to fully replicate this behavior you would need to set the commands dynamically each time, in case the time zone changed (laptops in particular tend to move time zones a lot). Here is what we currently do to get psql (which does support running commands before a REPL) to use the local time zone: #!/usr/bin/env bash
tz_data_raw=$(strings /etc/localtime| tail -1)
IFS=',' read -r -a tz_data <<< "$tz_data_raw"
URL="postgresql://$SQL_ADMIN_USER:$SQL_ADMIN_PW@$SQL_DB_HOST:$SQL_DB_PORT/$SQL_DB_NAME$SQL_URL_EXTRA"
psql "$URL" -c "set time zone ${tz_data[0]}" "$@" As you can see, it's not a great solution. As a compromise, maybe we can make this opt-in with a config parameter? I am generally against random configuration parameters but if there is an impasse I'd rather do that than have to make each member of my team install a fork. Thoughts
Experiment: Are Datetimes In Different Time Zones Equal?Postgres:
JavaScript: > const x = new Date("2024-10-17 00:00:00-07")
> const y = new Date("2024-10-17 01:00:00-06")
> x - y === 0
true
> x >= y && x <= y
true
> x === new Date("2024-10-17 00:00:00-07")
// this is to show that `===` is referential equality here for some reason
false Python: >>> x
datetime.datetime(2024, 10, 16, 0, 0, tzinfo=datetime.timezone(datetime.timedelta(days=-1, seconds=61200), 'PDT'))
>>> x.astimezone(timezone(timedelta(0)))
datetime.datetime(2024, 10, 16, 7, 0, tzinfo=datetime.timezone.utc)
>>> x == x.astimezone(timezone(timedelta(0)))
True |
pgcli/main.py
Outdated
@@ -1624,6 +1625,8 @@ def cli( | |||
else: | |||
pgcli.connect(database, host, user, port) | |||
|
|||
pgcli.pgexecute.set_timezone(tzlocal.get_localzone_name()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this ever raise an exception or return a None?
What happens if I set TZ
to a non-existent timezone name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- It does not throw if
TZ
is an invalid value (I also tested this directly) - It would throw if multiple conflicting timezone configurations are found in one of the above files. I added a try-catch to print a warning and continue without setting a custom timezone (using the default server settings instead)
- It might return
None
if no timezone configuration is found. I added a check that prints a warning and continues without setting a timezone
For due diligence, from reading the source code:
- It will throw if
TZ
points to a file (absolute path) andos.path.exists
orrealpath
return throw an error - It could throw if
exists("/system/bin/getprop")
throws - It could throw if
exists
,islink
orrealpath
throws for/etc/localtime
- It could throw if
realpath
throws for a timezone config referred to from/etc/timezone
,/var/db/zoneinfo
,/etc/sysconfig/clock
,/etc/conf.d/clock
, or/etc/localtime
Signed-off-by: maximsmol <[email protected]>
Signed-off-by: maximsmol <[email protected]>
Description
Sets the session time zone to the system time zone after connecting.
Adds
tzlocal
as a dependency for a cross-platform way of getting the current time and handling theTZ
environment variable. I've considered including code for this directly but we need to handle a lot of different Windows quirks. This package adds 126KB to the on-disk installation size.Checklist
changelog.rst
.AUTHORS
file (or it's already there).pip install pre-commit && pre-commit install
), and ranblack
on my code.