fix: keep legacy 200/400 codes for older clients in schedule-related endpoints - #2429
Conversation
…endpoints Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Documentation build overview
6 files changed ·
|
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
|
|
||
| Default: ``None`` | ||
|
|
||
| FLEXMEASURES_LEGACY_SCHEDULEACCEPTED_STATUS_MAX_INCOMPATIBLE_CLIENT_VERSION |
There was a problem hiding this comment.
I suggest introducing one less configuration setting, by combining the two:
FLEXMEASURES_LEGACY_SCHEDULEACCEPTED_STATUS: {"v2g-liberty-version": "0.9.1"}
This also makes it possible to have more than one entry for matching.
Better would be to match the version with the actual v2g-liberty version, and only use the fm-client version if that's the actual attribute.
There was a problem hiding this comment.
I agree, having only one setting is easier, and mostly allowing more than one attribute to be used helps to support more partners with client systems.
Better would be to match the version with the actual v2g-liberty version, and only use the fm-client version if that's the actual attribute.
I don't understand what you mean actually. We don't know the "actual" v2g-liberty version aside from what these users put in the asset attributes. There is no fm-client version that we know of - I will prioritize FlexMeasures/flexmeasures-client#212 so that we start collecting it, but that is not relevant to FM v1.0.
There was a problem hiding this comment.
I thought @ArdJonker was storing both the v2g-liberty version and the flexmeasures-client version as asset attributes on the main connection asset (the one that a schedule is triggered on).
And I thought it would make sense to not mix up versions in our new config setting. So, for instance, if they store:
v2g-liberty-version: 0.8.0
flexmeasures-client: v0.9.1
we'd not want a setting like:
v2g-liberty-version: 0.9.1
because the value and key then represent different dependencies.
Rather, I was thinking of:
FLEXMEASURES_LEGACY_SCHEDULEACCEPTED_STATUS:
v2g-liberty-version: 0.8.0
to apply legacy status on all assets with their v2g-liberty-version attribute set to lower than 0.8.0, or
FLEXMEASURES_LEGACY_SCHEDULEACCEPTED_STATUS:
flexmeasures-client: v0.9.1
to apply legacy status on all assets with their flexmeasures-client attribute set to lower than v0.9.1, or
FLEXMEASURES_LEGACY_SCHEDULEACCEPTED_STATUS:
v2g-liberty-version: 0.8.0
flexmeasures-client: v0.9.1
to apply legacy status on all assets with their v2g-liberty-version attribute set to lower than 0.8.0 or their flexmeasures-client attribute set to lower than v0.9.1.
Footnotes:
- Code blocks in YAML just for quicker typing on mobile; they should have been JSON, really.
- I didn't look up the correct versions for these notes.
- The version comparison should probably be made robust against both "v0.9.1" and "0.9.1" values.
- The "lower than" part is still implicit and might be preferred to be made explicit, for instance:
flexmeasures-client: <0.9.1.
There was a problem hiding this comment.
Actually, from what I see he is storing:
v2g-liberty-version: 0.8.0
home-assistant-version: 2026.8.0
The former impicitly would tell us which fm-client is used.
There was a problem hiding this comment.
I checked about the "v" robustness, and we use packaging.version.Version, which handles that correctly.
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
…-status' of github.com:FlexMeasures/flexmeasures into fix/backward-compatibility-switch-for-schedule-accepted-status
Extend the existing job-response entry so that an upgrading host learns that polling a schedule whose job has not finished yet now returns 202 Accepted, and how to opt individual assets back in to the previous status codes. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
BelhsanHmida
left a comment
There was a problem hiding this comment.
Approved.
Tested locally against a real server with actual flexmeasures-client releases:
0.9.1 gets the legacy 200/400 codes and a valid schedule when tagged resolving at
the device, parent, and grandparent level and 0.9.5 is unaffected whether an asset
is tagged or not, so tagging is safe to roll out broadly. The full HEMS walkthrough
completes on this branch. The one-dict redesign reads well, and tolerating a
non-mapping value is a good call.
Pushed a one-line changelog tweak so an upgrading host also learns that polling an
unfinished schedule now returns 202, with a pointer to the opt-in setting.
One thing I wanted to get your thoughts on:
_get_asset_attribute_from_nearby_hierarchystops at the first asset that
contains the attribute key, so a device with a null value shadows a correctly
tagged site and silently disables the shim.edit attribute --nullproduces
exactly that, and it's the only way the CLI can clear an attribute. It's worth a
look because the failure is invisible: the legacy client returns a dict with no
valuesand raises nothing. Skipping falsy values fixes it — though that also
means a device could no longer use null to opt out of its site's setting, so
it's your call whether null should mean "inherit" or "explicitly off".
If you consider that behaviour acceptable, then this is approved as it stands.
Otherwise I'm happy to push the fix.
|
Interesting point, even though not high-iimpact I think. As you point out, each approach has a draw-back. But opting-out from a site's setting seems less needed. Let's make the change you suggest! |
Walking the nearby asset hierarchy stopped at the first asset that merely contained the configured attribute key, so a device carrying a null or empty value disabled the legacy schedule shim even when its site had a valid version. Skip such values and keep looking up the hierarchy, which makes a null on a device mean "inherit from the site" rather than "explicitly off". Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
pushed. |
Description
There are users using flexmeasures-client before 0.9.2, and they will have trouble getting their schedules when we release v1.0.0.
We switched to the correct way of returning HTTP status 202 when the job is not done. An earlier version of FlexMeasures returned 200 and the FM client (until 0.9.2) will only poll for results when it sees the combination of code 200 together with some specific messages it tests for (e.g. "Request has been accepted for processing.")
Note: the client also supports another combiinatiion: status code 503 and a "Retry-After" header, but the FM server never implemented this, actually.
Done in this PR
documentation/changelog.rstHow to test