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

433 patch sensor #773

Merged
merged 28 commits into from Aug 2, 2023
Merged

433 patch sensor #773

merged 28 commits into from Aug 2, 2023

Conversation

GustaafL
Copy link
Contributor

@GustaafL GustaafL commented Jul 25, 2023

Description

Adds an API endpoint to PATCH a sensor

Look & Feel

endpoint sensors/<id>

json:
{
"name": "POWER",
}

response:
{
"name": "POWER",
"event_resolution": "PT1H",
"unit": "kWh",
"generic_asset_id": 1,
}

Related Items

Relates to issue #433

Mention if this PR closes an Issue or Project.


  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on code under GPL or other license that is incompatible with FlexMeasures

GustaafL and others added 13 commits July 5, 2023 14:47
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
…corator, especially arg_loader was misleading

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
…or, for when we only have an AuthModelMixin ID.

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
flexmeasures/api/v3_0/sensors.py Show resolved Hide resolved
flexmeasures/api/v3_0/tests/test_sensors_api.py Outdated Show resolved Hide resolved
Base automatically changed from 433-post-sensor to main August 1, 2023 08:19
@GustaafL GustaafL marked this pull request as ready for review August 1, 2023 15:54
@GustaafL GustaafL requested a review from nhoening August 1, 2023 15:54
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

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

Looks really good!

I have one concern about formalizing what attributes are excluded in the PartialSchema and how. If we get that straight (by changes or discussion), this can be merged.

documentation/api/change_log.rst Outdated Show resolved Hide resolved
flexmeasures/api/v3_0/sensors.py Outdated Show resolved Hide resolved
flexmeasures/api/v3_0/sensors.py Show resolved Hide resolved
flexmeasures/api/v3_0/sensors.py Show resolved Hide resolved
GustaafL added 4 commits August 2, 2023 15:36
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
…oved docs

Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

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

Thanks!

I think this is already fine, with one caveat:

As a frontend dev, I might want to get the sensor ID back from fetch_one and patch (in order to build valid API URLs for the sensor for the next action on it). I don't think that's highly critical, so it could also go into a follow-up ticket.

The GenericAssetSchema has this line, maybe adding this to the SensroSchema would accomplish this:

id = ma.auto_field()

If that works superwell, we could include it here, otherwise make a follow-up issue.

@GustaafL
Copy link
Contributor Author

GustaafL commented Aug 2, 2023

The GenericAssetSchema has this line, maybe adding this to the SensroSchema would accomplish this:

id = ma.auto_field()

If that works superwell, we could include it here, otherwise make a follow-up issue.

I've added id = ma.auto_field(dump_only) and a test that it can not be edited.

Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

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

Is the ID now returned in fetch_one and /or patch?

@GustaafL
Copy link
Contributor Author

GustaafL commented Aug 2, 2023

All responses that return the sensor include the id. So will also need to fix the docstrings.

GustaafL and others added 2 commits August 2, 2023 21:14
Signed-off-by: GustaafL <guus@seita.nl>
* feat(sensor): adds delete sensor endpoint

Signed-off-by: GustaafL <guus@seita.nl>

* docs(sensor): updated api changelog

Signed-off-by: GustaafL <guus@seita.nl>

* docs(sensor): updated api changelog whitespace

Signed-off-by: GustaafL <guus@seita.nl>

* docs(sensor): updated api changelog missing /

Signed-off-by: GustaafL <guus@seita.nl>

* docs(sensor): update changelog

Signed-off-by: GustaafL <guus@seita.nl>

* docs(sensor): update changelog typo

Signed-off-by: GustaafL <guus@seita.nl>

* docs(sensor): update changelog missing space

Signed-off-by: GustaafL <guus@seita.nl>

* tests(sensor): check for float instead of exact value

Signed-off-by: GustaafL <guus@seita.nl>

---------

Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <41048720+GustaafL@users.noreply.github.com>
@nhoening nhoening merged commit 74dbbcc into main Aug 2, 2023
4 checks passed
@nhoening nhoening deleted the 433-patch-sensor branch August 2, 2023 21:40
@GustaafL GustaafL added this to the 0.15.0 milestone Aug 3, 2023
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