Skip to content

RFC 1123 date format fix#30125

Merged
simorenoh merged 4 commits intoAzure:mainfrom
bambriz:issue-29109-RFC-1123-Dates
May 1, 2023
Merged

RFC 1123 date format fix#30125
simorenoh merged 4 commits intoAzure:mainfrom
bambriz:issue-29109-RFC-1123-Dates

Conversation

@bambriz
Copy link
Copy Markdown
Member

@bambriz bambriz commented Apr 27, 2023

Description

This fixes an issue with non-English locales making non-RFC 1123 Date formats. This fix makes it into an RFC 1123 Date format regardless of the locale. See Issue 29109

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

This fixes an issue with non english locales making non RFC 1123 Date formats. This fix makes it into a RFC 1123 Date format regardless of the locale.
update changelog to include PR link
@azure-sdk
Copy link
Copy Markdown
Collaborator

API change check

API changes are not detected in this pull request.

Comment thread sdk/cosmos/azure-cosmos/azure/cosmos/_base.py Outdated
@simorenoh
Copy link
Copy Markdown
Member

Question, why are we re-inventing the wheel here? Is there no existing native package we can use? I'd think we would want to avoid any manual parsing or creating of day/month abbreviations if we don't exclusively need them.

Updated to use emails.utils formatdate as it does exactly what the manual implementation would have done.
@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Apr 28, 2023

Question, why are we re-inventing the wheel here? Is there no existing native package we can use? I'd think we would want to avoid any manual parsing or creating of day/month abbreviations if we don't exclusively need them.

Changed to use email.utils formatdate.

@simorenoh
Copy link
Copy Markdown
Member

/azp run python - cosmos - tests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Member

@simorenoh simorenoh left a comment

Choose a reason for hiding this comment

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

LGTM thanks Bryan - seems like we will have to disable the pkdelete test while we update the subscription being used for our pipeline test account. I'll make an issue for that and assign you to it.

Currently pipeplines dont support this feature
@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented May 1, 2023

/azp run python - cosmos - tests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@simorenoh simorenoh merged commit fc98f3b into Azure:main May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants