FlexMeasures / flexmeasures

The intelligent & developer-friendly EMS to support real-time energy flexibility apps, rapidly and scalable.
https://flexmeasures.io
Apache License 2.0
133 stars 34 forks source link

Fix deleting beliefs in CLI #1095

Open Ragnar-the-mighty opened 3 weeks ago

Ragnar-the-mighty commented 3 weeks ago

fixes #1092 1092

There are more places that has same issues but I do not have test cases so I only fixed this one.

Description

The cli command flexmeasures delete prognoses returns TypeError: %d format: a real number is required, not CursorResult

The reason is that after move to SQLAlchemy 2 the code num_forecasts_deleted = db.session.execute(query) Does not work as it use to do.

The fix is to use .rowcount as recommended by SQLAlchemy 2 documentation

Look & Feel

No change in UI No change in CLI parameter or API endpoints

How to test

Call the cli command flexmeasures delete prognoses It should no longer throw the TypeError: %d format: a real number is required, not CursorResult

Further Improvements

There are more instances of num_forecasts_deleted = db.session.execute(query) But I am a newbee and I do not have test cases for the other instances.

Related Items

This should close issues #1092


Ragnar-the-mighty commented 3 weeks ago

OK, I try to fix the other instances and the also the flexmeasures delete beliefs command

nhoening commented 3 weeks ago

Thanks!

Ragnar-the-mighty commented 2 weeks ago

Hmm, I did not change flexmeasures/cli/data_delete.py and the flexmeasures delete beliefs command work for me.

I am not trusting my code changes and I do not want to cause work for anyone else. I appreciate the help I got to unblock myself. I will suspend work on PR until I have more confidence that I understand what I am doing.

nhoening commented 2 weeks ago

Yes, flexmeasures delete beliefs also works for me, not sure what the difference is. I also got flexmeasures delete unchanged-beliefs to work.

Then this PR should simply concentrate on flexmeasures delete measurements (which I tested to have the same bug you reported). Can you apply the fix there, we well? Then I believe we did a good step with this PR.

Ragnar-the-mighty commented 2 weeks ago

Nicolas, thank you for testing and merging the code, I hope that I will be able to do test better and fix any issues blocking merge

nhoening commented 2 weeks ago

I believe the remaining work would be to simply repeat what you did for one command in the other one.