Skip to content

Conversation

@mikijov
Copy link
Contributor

@mikijov mikijov commented Jul 12, 2019

Some non Amazon S3 implementations are not consistent about including fractional seconds in the timestamp. The change allows parsing of both 2019-07-10T14:23:39.231Z and 2019-07-10T14:23:39Z.

@nitisht nitisht requested review from ebozduman and vadmeste July 12, 2019 15:40
@harshavardhana
Copy link
Member

harshavardhana commented Jul 12, 2019

Can you state which Amazon S3 implementation is this ? Please mention them in the comment. We may have to remove this code in future, as this should be seen as a workaround.

@mikijov
Copy link
Contributor Author

mikijov commented Jul 12, 2019

Added comment into the code. I encountered this issue on Dell EMC ECS.

Copy link
Collaborator

@ebozduman ebozduman left a comment

Choose a reason for hiding this comment

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

LGTM

@mikijov
Copy link
Contributor Author

mikijov commented Aug 21, 2019

Build job failed due to some timeout, I have to close/re-open the PR to trigger the build.

@mikijov mikijov closed this Aug 21, 2019
@mikijov mikijov reopened this Aug 21, 2019
@mikijov
Copy link
Contributor Author

mikijov commented Aug 21, 2019

Hi @ebozduman, @vadmeste, are there any concerns merging this? We actually need this in production, and we are struggling patching minio-py on the fly...

Copy link
Member

@vadmeste vadmeste 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
Collaborator

@ebozduman ebozduman left a comment

Choose a reason for hiding this comment

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

LGTM

@mikijov
Copy link
Contributor Author

mikijov commented Aug 21, 2019

Thanks for approval. Will someone be able to merge it?

@kannappanr kannappanr merged commit 8d8bf07 into minio:master Aug 22, 2019
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.

5 participants