Skip to content

Misc fixes for client.py#2

Merged
tjhorner merged 5 commits intotjhorner:mainfrom
cdce8p:fix-client
Sep 14, 2023
Merged

Misc fixes for client.py#2
tjhorner merged 5 commits intotjhorner:mainfrom
cdce8p:fix-client

Conversation

@cdce8p
Copy link
Contributor

@cdce8p cdce8p commented Sep 14, 2023

  • Fix typing: Replace any (the function) with typing.Any
  • Remove OrderedDict: for newer Python (>=3.7) versions the insertion order into dicts is already kept
  • Move default arguments for hourly_start and hourly_end into the function body. Otherwise they are executed when the the class is initialized. I.e. those arguments would stay the same for new method calls.
  • Replace datetime.utcnow with the timezone aware datetime.now(tz=UTC).
    utcnow will be deprecated with Python 3.12.

I wasn't sure about the + "Z" which you added manually to the iso datetime. Could you check which format the API expects? datetime.isoformat would return YYYY-MM-DDTHH:MM:SS.ffffff+HH:MM.
https://docs.python.org/3/library/datetime.html#datetime.datetime.isoformat

@tjhorner
Copy link
Owner

I wasn't sure about the + "Z" which you added manually to the iso datetime. Could you check which format the API expects? datetime.isoformat would return YYYY-MM-DDTHH:MM:SS.ffffff+HH:MM.

Passing a date in that format (I used 2023-09-14T16:09:38.522987+00:00) results in a 400 from the API. Passing the same date in the previous format (2023-09-14T16:09:38.522987Z) works fine. The documentation never explicitly mentions a date format to use (the scalar type is just date-time and there is no description of this type anywhere), but the responses all use UTC dates in the format YYYY-MM-DDTHH:MM:SSZ.

@cdce8p
Copy link
Contributor Author

cdce8p commented Sep 14, 2023

Passing a date in that format (I used 2023-09-14T16:09:38.522987+00:00) results in a 400 from the API. Passing the same date in the previous format (2023-09-14T16:09:38.522987Z) works fine. The documentation never explicitly mentions a date format to use (the scalar type is just date-time and there is no description of this type anywhere), but the responses all use UTC dates in the format YYYY-MM-DDTHH:MM:SSZ.

Added some logic to convert the datetime arguments to UTC (if they aren't already) and to remove the timezone afterwards. Note that it will only convert to UTC if the datetime was passed with a timezone, otherwise it would assume UTC. Does this make sense?

@tjhorner
Copy link
Owner

Sounds good to me.

@tjhorner tjhorner merged commit e6ab534 into tjhorner:main Sep 14, 2023
@cdce8p cdce8p deleted the fix-client branch September 14, 2023 22:48
@tjhorner
Copy link
Owner

Released this as 1.0.3.

@cdce8p Do you anticipate making any more PRs soon? If not, I will go ahead and create the PR to bump the version in HA core.

@cdce8p
Copy link
Contributor Author

cdce8p commented Sep 14, 2023

@cdce8p Do you anticipate making any more PRs soon? If not, I will go ahead and create the PR to bump the version in HA core.

No 😉
Just saw the DeprecationWarning in Home Assistant and wanted to fix it. I've already started testing Python 3.12 on my fork.

@tjhorner
Copy link
Owner

Great, thanks! I'm not too familiar with Python and its ecosystem/idiosyncrasies so I appreciate you catching and fixing these.

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