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 RFC3339 representation #150

Merged
merged 2 commits into from
May 15, 2018
Merged

Fix RFC3339 representation #150

merged 2 commits into from
May 15, 2018

Conversation

marcelstoer
Copy link
Contributor

https://now.httpbin.org/ returns an invalid RFC3339 representation. It currently shows 2-digit milliseconds. I hope I tracked it correctly back to Maya core.py.

RFC3339 5.6 allows at most 1 digit for milliseconds: https://tools.ietf.org/html/rfc3339#section-5.6.

time-secfrac    = "." 1*DIGIT
partial-time    = time-hour ":" time-minute ":" time-second [time-secfrac]

I'm not 100% happy with the fix as .%f apparently isn't guaranteed to always return 6 digits (platform dependent). https://stackoverflow.com/a/35643540/131929 shows a more stable version based on splitting and reformatting the original string.

@timofurrer
Copy link
Collaborator

Thanks for the contribution and sorry for the long wait.

The change looks good. Would you be up for writing a test to ensure that the RFC3339 is correctly implemented?

🎉

@marcelstoer
Copy link
Contributor Author

Sure, I try not to forget it.

@timofurrer
Copy link
Collaborator

Great! Ready when you are 🎉

@marcelstoer
Copy link
Contributor Author

I get 10 failed tests on macOS before I even start (also on master). This is regardless of whether I do pipenv run pytest tests/ or simply run make. Are the tests supposed to pass on macOS at all?

@timofurrer
Copy link
Collaborator

Are the tests supposed to pass on macOS at all?

Yes, they should.

Can you open a new issue with some more information about the test failures you get?

Must be at most 1-digit millisecond.
@timofurrer
Copy link
Collaborator

Thanks for the contribution 🎉

@timofurrer timofurrer merged commit 62a6283 into kennethreitz:master May 15, 2018
@timofurrer
Copy link
Collaborator

I've released v0.4.3 containing this fix!

@marcelstoer
Copy link
Contributor Author

To fix the originally reported problem someone would have to deploy that latest version for https://now.httpbin.org/

@timofurrer
Copy link
Collaborator

That "someone" would be Kenneth. I see what I can do.

@marcelstoer
Copy link
Contributor Author

@kennethreitz - ping, how about upgrading httpbin.org?

@marcelstoer marcelstoer deleted the patch-1 branch June 15, 2018 14:46
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.

2 participants