-
Notifications
You must be signed in to change notification settings - Fork 36
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
Fix deleting beliefs in CLI #1095
base: main
Are you sure you want to change the base?
Fix deleting beliefs in CLI #1095
Conversation
After move to SQLAlchemy 2 the code num_forecasts_deleted = db.session.execute(query) Does not work as it use to. Changed code to use .rowcount There are more places that has same issues but I do not have test cases so I only fixed this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for helping to fix this!
Let's fix the other instances as well, and also the flexmeasures delete beliefs
command I mentioned in #1092
We can do this, especially because with the help of this method of adding data we can create whatever we want, then attempt to delete it :)
OK, I try to fix the other instances and the also the flexmeasures delete beliefs command |
Thanks! |
Hmm, I did not change flexmeasures/cli/data_delete.py and the I am not trusting my code changes and I do not want to cause work for anyone else. |
Yes, Then this PR should simply concentrate on |
…lete-prognoses-fails-with-TypeError-
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 |
I believe the remaining work would be to simply repeat what you did for one command in the other one. |
Hi @Ragnar-the-mighty are you able to fix the other command we talked about as well ( |
…lete-prognoses-fails-with-TypeError-
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