-
Notifications
You must be signed in to change notification settings - Fork 20
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
Issue 147 postgres integration #297
base: main
Are you sure you want to change the base?
Issue 147 postgres integration #297
Conversation
…greSQL using matrix note: Setted up chart github url to point to my fork, MUST change later
…imezone Mysql removes the timezone before inserting, but Postgresql throws an error
SubmissionTime = NullColumn("SubmissionTime", DateTime(timezone=True)) | ||
RescheduleTime = NullColumn("RescheduleTime", DateTime(timezone=True)) | ||
LastUpdateTime = NullColumn("LastUpdateTime", DateTime(timezone=True)) | ||
StartExecTime = NullColumn("StartExecTime", DateTime(timezone=True)) | ||
HeartBeatTime = NullColumn("HeartBeatTime", DateTime(timezone=True)) | ||
EndExecTime = NullColumn("EndExecTime", DateTime(timezone=True)) |
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.
Why? We are anyway always using UTC.
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.
Is because we are specifying the timezone in datetime.now(tz=timezone.utc)
.
While this does not throw any errors in MySQL when the table does not accept a timezone, in PostgreSQL stops the execution due to conflicting formatting.
One way of fixing it is just to completely remove the timezone on the datetime.now
function, which is not recommended and even RUFF marks it as a warning.
The other way is by telling the column to take into account the timezone, which in my opinion is the better way of implementing it.
MySQL:
- Changing the table property: 2024-09-26 14:00:32
- Removing
tz=timezone.utc
: 2024-09-26 14:05:53
No change, both result in the same date format.
PostgreSQL:
- Changing the table property: 2024-09-26 13:50:26.213893+00
- Removing
tz=timezone.utc
: 2024-09-26 13:55:57.840623
Timezone is or is not added at the end.
thanks a lot for the excellent work. We unfortunately cannot have timezone aware datetime in our DB as long as we use legacy DIRAC, as this would require too much change. This effectively mean that we cannot have Postgress for DBs which come from Legacy DIRAC as long as they are there. 2 options:
In any case, there's the priority is quite clear: first port all the services to diracx before trying to change any db schemas (which hopefully will just be an alambic job by then). @AcquaDiGiorgio just to make things clear : these problems have nothing to do with the PR you have done ! You've just helped us discovering these problems ! :-) |
Full integration with PostgreSQL
Used asyncpg as the driver because it is the fastest and one of the most used by the community
Had to change a few aspects that were MySQL specific. Those being unnamed Enums, which are not permitted in Postgres, and the way of returning the last inserted ID, which used result.lastrowid and postgres needs result.inserted_primary_key[0]