Skip to content

Conversation

@cebtenzzre
Copy link
Contributor

@cebtenzzre cebtenzzre commented Nov 8, 2025

There is no need to "clean" the json body of a POST request to alter things like booleans - JSON already has a way to represent them. That wasn't done in 4.4.x, which had a more transparent set_state.

One possible exception is datetime, which doesn't have one true JSON representation. But we can enhance convert_json to always use isoformat.

(Also, DELETE shouldn't have a body - but it may have query params. We don't currently use kwargs with DELETE so this isn't crucial, but it's important for correctness.)

Fixes #2464

@acockburn acockburn requested a review from jsl12 November 16, 2025 14:46
Copy link
Contributor

@jsl12 jsl12 left a comment

Choose a reason for hiding this comment

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

Using urllib to actually parse things should resolve this stuff about where the slashes are, so I don't think #2485 is necessary any more

@cebtenzzre
Copy link
Contributor Author

Well, that PR is about websocket_url and states_api in plugin.py, and this PR doesn't even touch that file. Are you looking for a urllib-based version of #2485?

Comment on lines 1176 to 1180
return result.strip("/")
parsed = parse.urlparse(str(base))
result = parsed._replace(path=endpoint)
return parse.urlunparse(result)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this breaks base URLs with a path (e.g. http://supervisor/core) - it will replace /core with the endpoint path instead of appending. See #2485 for a better fix using yarl.

cebtenzzre and others added 8 commits November 30, 2025 14:39
We need a serializer for all fields in the model.
We prefix paths like `/api/states` with a slash even though they're
relative in practice, not absolute. It's simpler to always append to the
path and lstrip the leading slash from the endpoint.
@acockburn acockburn merged commit 9f8dcd8 into AppDaemon:dev Nov 30, 2025
9 checks passed
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.

Regression on set_state api call

3 participants