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

fix(/tastypie/fields.py): Remove usage of datetime_safe module. #1662

Conversation

Pavankumardontha
Copy link

Fixes #1660

@Pavankumardontha
Copy link
Author

Hi @georgedorn . Can you please check if this PR would fix the issue. ?

@Pavankumardontha
Copy link
Author

@georgedorn Is there anything more that i need to do to fix the issue ??

@Pavankumardontha
Copy link
Author

@georgedorn can you please once check this PR and let me know this can fix the issue ?

@georgedorn
Copy link
Contributor

georgedorn commented Sep 5, 2023

I don't know if this is the correct fix. datetime_safe was used for a reason now lost to time, so simply swapping it back for the python native may expose a regression. My brief skimming of the issue the Django team was attempting to fix suggests that it has not been fixed in python and that for a variety of dates before 1900 (or 1000) this may break in unexpected ways. Worse, that breakage may vary by platform, and we don't have cross-platform test support.

At the very least, we need tests showing that this works for dates where the bug is known to occur. None of the current tests consider anything before 2010.

Allowing a DeprecationWarning is preferable to triggering a regression, so until 5.0 is released and forces the issue, merging blindly is not the best option.

@benrxv
Copy link

benrxv commented Dec 9, 2023

Django 5.0 is out now. I also don't know if replacing datetime_safe with python's native datetime is correct. But what I do know is that with the deprecation of datetime_safe, Django began sanitizing datetime data to check the format, and in at least one part of their code, django.utils.feedgenerator they did in fact do the simple replacement.

Django haystack explains (far better than I) why they think replacement is fine in their project here django-haystack/django-haystack#1915 and I believe they are documenting django 5.0 support as experimental.

@georgedorn
Copy link
Contributor

Implemented in #1666

@georgedorn georgedorn closed this Jan 18, 2024
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.

datetime_safe
4 participants