Feat: Cleanup stale assets in graphs and configs#2210
Conversation
…d improving error handling for asset fetching Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Ignore stale asset references in sensors_to_show for usability and automatic pruning when deleting assets. Signed-off-by: JDev <45713692+joshuaunity@users.noreply.github.com>
…r stale asset references Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
…exMeasures/flexmeasures into feat/stale-asset-references-in-graphs
…h database context Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
… deletion Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
nhoening
left a comment
nhoening
left a comment
There was a problem hiding this comment.
I re-did the initial setup, and the deleted asset reference was removed befire the graph page could even get in trouble, nice.
I need some examples in two utils, as they are not easy to grasp without. Then I know better what I approve.
Also, I ran into a separate but related problem: When a config setting is removed from its asset after it was added to some sensor_to_show, we also get an error when that graph dialogue is opened. Is that better to do in a follow-up PR?
…amples Signed-off-by: Joshua Edward <oghenerobojosh01@gmail.com>
nhoening
left a comment
nhoening
left a comment
There was a problem hiding this comment.
Thanks for the examples.
Please check failing tests, it might just have to do with quotes:
Expected:
[{"title": "Power", "plots": [{"sensor": 1}]}, {"title": "Price", "plots": [{"sensor": 3}]}]
Got:
[{'title': 'Power', 'plots': [{'sensor': 1}]}, {'title': 'Price', 'plots': [{'sensor': 3}]}]
And do check the diff - do you know why we are deleting a lot of code in flexmeasures/data/tests/test_belief_charts.py? That seems unrelated to this PR.
| try: | ||
| asset = GenericAssetIdField().deserialize(asset_id) | ||
| except ValidationError: | ||
| if self.allow_missing_asset_flex_field: |
There was a problem hiding this comment.
I can't find this field anywhere else in this PR - is this still relevant?
There was a problem hiding this comment.
it's in ln 440 in /data/models/generic_assets.py
This was implemented before the removal of the stale asset feature was completed. The goal for this is so that the graph page doesn't crash in the case of a stale asset. It's like a second net to safeguard the UI in a case where an asset somehow didnt get cleaned up.
There was a problem hiding this comment.
Ah thanks.
Do you recall why we let it default to True?
…r consistency Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
nhoening
left a comment
nhoening
left a comment
There was a problem hiding this comment.
This looks okay to me, I just have questions.
We can probably merge this today :)
| asset_id: int, asset_name: str | None = None | ||
| ) -> int: | ||
| """Remove references to a deleted asset in sensors_to_show across assets.""" | ||
| vars_json = sa.func.jsonb_build_object("aid", asset_id) |
There was a problem hiding this comment.
What is "aid"?
Maybe add a comment so I can understand the way you find candidates better.
There was a problem hiding this comment.
3/4 of the changes in this file are just reformatting, is that correct?
I believe you still have a formatter in your editor that has a different opinion and makes changes by itself.
2 participants
Description
documentation/changelog.rstLook & Feel
getAssetinflexmeasures/ui/static/js/ui-utils.jsnow throws descriptive errors on non-2xx responses, enabling robust fallback renderingHow to test
.venv/bin/python -m pytest flexmeasures/api/v3_0/tests/test_assets_api.py -k delete_asset_cleans_stale_asset_references_in_sensors_to_showsensors_to_showcontains an asset plot referencesensors_to_showsensors_to_showFurther Improvements
Related Items
Sign-off