-
Notifications
You must be signed in to change notification settings - Fork 63
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
Return timestamp values as datetime objects #67
Comments
@vshlapakov , @chekunkov , any toughts on this? |
As far as I understand you want this to happen transparently - if we know that endpoint has a timestamp field somewhere in the response we should covert it to datetime object. Several problems with that:
What I can propose is to provide and document some utility functions that will convert timestamps in milliseconds to/from datetime objects. This way we will both keep the client implementation simple and decoupled from the backend response format + provide a handy way to convert values to datetime objects for users. Regards accepting datetime objects as input parameters for client methods - it's feasible, I don't see any problems with that. |
I'm not particularly fond of adding an extra layer to process timestamps. |
We will need to add some complex response traversal rules to determine wheter value for the given field should be changed. If we do that simply by matching field name - we are risking to convert values defined by user, e.g. in spider args or in collections or in items. This check will negatively affect performance. Also I'm not sure it even worth it - I cannot recall any api client that does such conversion.
If we add new fields to the backend response we will not increase api version - at least with our current api versioning approach. And we don't want python client to be tightly coupled with backend response format, this will significantly increase cost of backend changes. Right now we simply decode json, json lines or msgpack response and I think that's what we should probably stick to. |
I do see
The python client then becomes a very thin layer on top of the API's JSON responses.
Do we know that in advance?
Other clients not doing it is not an argument for me. I don't understand why the python client should not try to make the lives of users a bit easier. |
IMO it's a minor improvement that requires major changes - potential advantages that transparent conversion to datatime objects has doesn't justify additional complexity it will bring to the client code, maintenance, we can spend developers time on something more important. |
Alright, I'll keep this open as nice to have then. |
I forgot add a comment on this. Yes, technically we can recursively check response for the given fields + check request url for endpoints like |
Several Scrapinghub API endpoints accept or return timestamps, currently as UNIX timestamp in milliseconds.
It would be great to have those values as
datetime.datetime
objects in the results so that consumers of python-scrapinghub calls do not have to convert them for interpretation.Passing
datetime.datetime
objects in methods allowing filtering on timestamps, e.g. wherestartts
andendts
arguments are supported, would be very handy too.The text was updated successfully, but these errors were encountered: