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

utils.strptime and utils.strftime are asymmetrical #81

Closed
prakashpp opened this issue Oct 19, 2018 · 7 comments
Closed

utils.strptime and utils.strftime are asymmetrical #81

prakashpp opened this issue Oct 19, 2018 · 7 comments

Comments

@prakashpp
Copy link

prakashpp commented Oct 19, 2018

EDIT: Current summary of this issue is here.


I am using python 3.7 and it looks like

DATETIME_FMT = "%04Y-%m-%dT%H:%M:%S.%fZ"
is not valid format for strptime. Am I missing something here?

@KAllan357
Copy link
Contributor

There's a bit of history with that, I guess this format is not supported on python 3.7.

#52
#63

For now its best to stay on 3.5.x unless there's some overriding reason not to be on that version of python.

@prakashpp
Copy link
Author

Ah ok, I was not aware about 3.7 support. So this is what I am facing

>>> from datetime import datetime
>>> from singer import utils
>>> import pytz
>>> utc = pytz.UTC
>>> now = datetime.utcnow().replace(tzinfo=utc)
>>> dtime = utils.strftime(now)
>>> utils.strptime(dtime)
ValueError: time data '2018-10-19T20:53:38.514627Z' does not match format '%Y-%m-%dT%H:%M:%SZ'

In short I can not get the datetime back with formatted string. The problem is because of

def strptime(dtime):
is not respecting DATETIME_FMT_MAC. I am not sure if this is python 3.7 specific (frankly I am python2 guy). I can do a workaround in my code to handle it but you think its an issue?

@prakashpp
Copy link
Author

Also locally I am running python 3.7 but this error is coming on stitchdata running on python 3.5. Let me know, happy to send a fix.

@timvisher
Copy link

I need to know some more details about the error your seeing.

  1. Can you provide logs for python 3.5 running into this issue?
  2. Can you tell me the platform details that you're seeing this on? Docker? Virtual Machine? AWS? What distribution are you using, how did you install python 3.5, etc.

I'm going to do a bit of digging myself but at the moment I suspect that the underlying C library is actually what's throwing this error.

@timvisher
Copy link

So it appears that you've uncovered an asymmetry between utils.strptime and utils.strftime. I'm unsure how long this asymmetry has existed but it's not great and unfortunately because that code hasn't changed for so long it's also not something that's going to be easy to fix.

That said, it appears if you use utils.striptime_to_utc it will DTRT.

Python 3.5.2 (default, Nov 23 2017, 16:37:01)
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from datetime import datetime
>>> from singer import utils
>>>
>>> utils.strptime_to_utc(utils.strftime(utils.now()))
datetime.datetime(2018, 10, 22, 16, 26, 15, 917887, tzinfo=<UTC>)
# => datetime.datetime(2018, 10, 22, 16, 26, 15, 917887, tzinfo=<UTC>)

We (or you via a PR) may take the step to deprecate utils.strptime entirely at some point. The presence of these 4 functions is certainly confusing. I believe based on an internal conversation that it's possible utils.strptime was informally deprecated.

@timvisher timvisher changed the title ValueError: '0' is a bad directive in format '%04Y-%m-%dT%H:%M:%S.%fZ' utils.strptime and utils.strftime are asymmetrical Oct 22, 2018
timvisher pushed a commit to timvisher/singer-python that referenced this issue Oct 22, 2018
Motivation
----------

This appears to have been deprecated for some time and is poorly behaved
in the case of fractional seconds. `utils.strptime_to_utc` is preferred
anyway.

See singer-io#81

Implementation Notes
--------------------

- Move to circle 2.0
- Add doctest to the nose runner
- Add a docstring and warn call to utils.strptime
@prakashpp
Copy link
Author

@timvisher thank you for the explanation, I will send a PR as soon I get time.

@timvisher
Copy link

#82 closes this issue.

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

No branches or pull requests

3 participants