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.

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

Select db dates at utc in replicate-changesets #726

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

AntonKhorev
Copy link
Contributor

@AntonKhorev AntonKhorev commented Jan 10, 2025

  • select now() gives you time with timezone
  • select created_at etc give you time without timezone because we don't store timezones
  • Time.parse() on time without timezone parses it as a local time
  • comparisons between times with/without timezones are broken both in ruby and in db queries

To fix all this I'm adding at time zone 'utc' to every time value from db that doesn't have a timezone.

@tomhughes
Copy link
Member

Presumably this is only a problem when running a machine in a non-UTC timezone?

@AntonKhorev
Copy link
Contributor Author

Probably, but there was one at time zone 'utc' before my changes, added in zerebubuth/openstreetmap-changeset-replication@6c2aee5. Why would that be needed if the goal was to make it run only in utc?

@tomhughes
Copy link
Member

My point was that as far as I know there are no problems in production, but that machine like all our machines runs in UTC all the time.

@AntonKhorev
Copy link
Contributor Author

Problems are going to happen in development. osm-website runs fine in non-utc timezones, replicate-changesets doesn't. Also replicate-changesets tries to work in database time by getting now from the database, but to stay in database time it shouldn't look at the timezone on ruby side. Time.parse looks at the timezone if the string it receives doesn't contain one.

@tomhughes
Copy link
Member

I guess this is fine then, thanks.

I do wonder if all these changes should be going in the upstream repo first? or are we treating that as dead now?

@tomhughes tomhughes merged commit 8788a7c into openstreetmap:master Jan 10, 2025
102 checks passed
@AntonKhorev
Copy link
Contributor Author

Even @zerebubuth changed the script here 60735f5 without updating https://github.com/zerebubuth/openstreetmap-changeset-replication

@AntonKhorev AntonKhorev deleted the at-timezone-utc branch January 11, 2025 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants