Skip to content

[18.0][IMP] queue_job: avoid deprecation warning about datetime.datetime.utcnow() in _odoo_now()#813

Merged
OCA-git-bot merged 1 commit intoOCA:18.0from
acsone:18.0-queue_job_odoo_now
Sep 18, 2025
Merged

[18.0][IMP] queue_job: avoid deprecation warning about datetime.datetime.utcnow() in _odoo_now()#813
OCA-git-bot merged 1 commit intoOCA:18.0from
acsone:18.0-queue_job_odoo_now

Conversation

@acsonefho
Copy link
Contributor

datetime.datetime.utcnow() is now deprecated and should be replaced by datetime.datetime.now() (optional TZ parameter).
As the original _odoo_now() doesn't contain the Timezone, the parameter datetime.UTC is not added into this improvement to stay a naive datetime.

The purpose of _odoo_now() is to have the current datetime in UTC timezone but staying naive datetime.

https://docs.python.org/3/library/datetime.html#datetime.datetime.utcnow

https://docs.python.org/3/whatsnew/3.12.html#:~:text=datetime%3A%20datetime,gh%2D103857.)

Original warning:
image

@OCA-git-bot
Copy link
Contributor

Hi @guewen,
some modules you are maintaining are being modified, check this out!

Copy link
Member

@amh-mw amh-mw left a comment

Choose a reason for hiding this comment

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

Given https://www.odoo.com/documentation/18.0/administration/on_premise/source.html#python

Odoo [18] requires Python 3.10 or later to run.

I went through the Python documentation for versions 3.10 through 3.13 and the guidance on replacing utcnow is consistent:

Warning: Because naive datetime objects are treated by many datetime methods as local times, it is preferred to use aware datetimes to represent times in UTC. As such, the recommended way to create an object representing the current time in UTC is by calling datetime.now(timezone.utc).

So shouldn't the change be

-    dt = datetime.datetime.utcnow()
+    dt = datetime.datetime.now(timezone.utc)

@acsonefho
Copy link
Contributor Author

Given https://www.odoo.com/documentation/18.0/administration/on_premise/source.html#python

Odoo [18] requires Python 3.10 or later to run.

I went through the Python documentation for versions 3.10 through 3.13 and the guidance on replacing utcnow is consistent:

Warning: Because naive datetime objects are treated by many datetime methods as local times, it is preferred to use aware datetimes to represent times in UTC. As such, the recommended way to create an object representing the current time in UTC is by calling datetime.now(timezone.utc).

So shouldn't the change be

-    dt = datetime.datetime.utcnow()
+    dt = datetime.datetime.now(timezone.utc)

Yes but if I do that, the returned datetime contains a timezone (and others errors occurs) due to difference between datetime and naive datetime.

@sbidoul
Copy link
Member

sbidoul commented Aug 14, 2025

Yes but if I do that, the returned datetime contains a timezone (and others errors occurs) due to difference between datetime and naive datetime.

Yet dt = datetime.datetime.now() is not correct. This still needs work.

@acsonefho
Copy link
Contributor Author

What do you think of this: datetime.datetime.now(datetime.timezone.utc).replace(tzinfo=None)

@sbidoul
Copy link
Member

sbidoul commented Aug 14, 2025

Yes, that sounds better.

@acsonefho acsonefho force-pushed the 18.0-queue_job_odoo_now branch from 8ed15a5 to 22c9da3 Compare August 14, 2025 13:23
Copy link
Member

@amh-mw amh-mw left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@benwillig benwillig left a comment

Choose a reason for hiding this comment

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

LGTM

@sbidoul
Copy link
Member

sbidoul commented Aug 14, 2025

I checked that _odoo_now() does the same as SELECT EXTRACT(EPOCH FROM NOW()); in psql.

That said, so does time.time(), so this can probably be simplified.

@acsonefho
Copy link
Contributor Author

I checked that _odoo_now() does the same as SELECT EXTRACT(EPOCH FROM NOW()); in psql.

That said, so does time.time(), so this can probably be simplified.

The current return type of _odoo_now() is datetime and time.time() is float.
That update probably needs others changes.

Copy link
Member

@FrancoMaxime FrancoMaxime left a comment

Choose a reason for hiding this comment

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

LGTM: code review

Comment on lines +191 to +192
# Need naive datetime
dt = datetime.datetime.now(datetime.timezone.utc).replace(tzinfo=None)
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @sbidoul

We can remove the funciton _datetime_to_epoch and directly return:

Suggested change
# Need naive datetime
dt = datetime.datetime.now(datetime.timezone.utc).replace(tzinfo=None)
return time.time()

I tested localy and in fact, _odoo_now() returns a float:

>>> dt = datetime.datetime.now(datetime.timezone.utc).replace(tzinfo=None)
>>> (dt - datetime.datetime(1970, 1, 1)).total_seconds()
1757943419.659445
>>> time.time()
1757943426.0511284
>>> 

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @AnizR comment

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @sbidoul

We can remove the funciton _datetime_to_epoch and directly return:

I tested localy and in fact, _odoo_now() returns a float:

>>> dt = datetime.datetime.now(datetime.timezone.utc).replace(tzinfo=None)
>>> (dt - datetime.datetime(1970, 1, 1)).total_seconds()
1757943419.659445
>>> time.time()
1757943426.0511284
>>> 

As discussed with @acsonefho, I have done the refactor of the function _odoo_now().

Copy link

@SirPyTech SirPyTech left a comment

Choose a reason for hiding this comment

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

Thanks!

@AnizR AnizR force-pushed the 18.0-queue_job_odoo_now branch from 22c9da3 to 95ebb0f Compare September 17, 2025 14:17
def _odoo_now():
dt = datetime.datetime.utcnow()
return _datetime_to_epoch(dt)
return time.time()
Copy link
Member

Choose a reason for hiding this comment

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

I'd keep the comment that says it must do the same as the postgres equivalent, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, done 👍

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

datetime.datetime.utcnow() is now deprecated and should be replaced by datetime.datetime.now() (optional TZ parameter).
As the original _odoo_now() doesn't contain the Timezone, the parameter datetime.UTC is not added into this improvement
@AnizR AnizR force-pushed the 18.0-queue_job_odoo_now branch from 95ebb0f to 26e94fc Compare September 17, 2025 14:26
@guewen
Copy link
Member

guewen commented Sep 18, 2025

/ocabot merge patch

OCA-git-bot added a commit that referenced this pull request Sep 18, 2025
Signed-off-by guewen
@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-813-by-guewen-bump-patch, awaiting test results.

@OCA-git-bot
Copy link
Contributor

It looks like something changed on 18.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 18.0-ocabot-merge-pr-813-by-guewen-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit bb14a6e into OCA:18.0 Sep 18, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 0a2be50. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants