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

feat: add tz info to all naive datetime object #120

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ranjanrak
Copy link
Member

@ranjanrak ranjanrak commented Jul 1, 2021

1> Add IST timezone info to all naive DateTime objects.
Eg: 'order_timestamp': datetime.datetime(2021, 7, 1, 16, 45, 36, tzinfo=tzoffset('Asia/Kolkata', 19800)), 'exchange_timestamp': datetime.datetime(2021, 7, 1, 16, 45, 36, tzinfo=tzoffset('Asia/Kolkata', 19800)
2> Introduce proper logic(is_timestamp) to check if the response string is a timestamp field. Give away with an earlier string length comparison.
3> Add optional mode field for order_margins.
4> Add example with mode param for order_margins.
5> Change the way to handle exceptions for error responses, which don't have the error_type field(MF APIs).
Ex: Kite error response is: {'status': 'error', 'message': "Couldn't find that order_id.", 'data': None, 'error_type': 'GeneralException'}
Where MF error response is: {'status': 'error', 'message': 'Order not found', 'data': {}}. Don't have error_type field in response.

Copy link
Member

@vividvilla vividvilla left a comment

Choose a reason for hiding this comment

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

Also this will be breaking change so we need to release it as 3.10.0 or 4.0.0, we can decide on this later.

kiteconnect/connect.py Show resolved Hide resolved
kiteconnect/connect.py Show resolved Hide resolved
kiteconnect/connect.py Outdated Show resolved Hide resolved
kiteconnect/connect.py Outdated Show resolved Hide resolved
kiteconnect/connect.py Show resolved Hide resolved
kiteconnect/connect.py Show resolved Hide resolved
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.

None yet

2 participants