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

remove flask upper pin #411

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

utnapischtim
Copy link
Contributor

@utnapischtim utnapischtim commented Oct 8, 2024

  • fix: tests
  • refactor: reduce queries
  • fix: deprecation warning
  • compatibility: sqlalchemy, flask-sqlalchemy
  • tests: relations handling, forward compatibility
  • tests: forward compatibility

NOTE:
the last two commits should be discussed

* the test raises a permission denied error. this is then handled by the
  UOW by rolling back the session. this rollback also removes the test
  data from the session. the testdata is then not existing anymore for the
  following up test cases. moving those three in different test functions
  solves the problem.

* another solution would be to find out to only roll back the last
  savepoint but at the moment of the rollback no savepoint is open so the
  whole session will be rolledback.
* the problem was that with sqlalchemy some strange rollback happened
  since the exception was raised.

* with this change the re raise is not necessary anymore
* create a forward change to be compatible with sqlalchemy >= 2.0 and
  flask-sqlalchemy >= 3.0

* the biggest change is to go away from commit driven approach. in
  sqlalchemy it is more done with transactions.

* a explicit call of a commit triggers a COMMIT event and writes to the
  database and therefore it couldn't be rolledback anymore
* this change should not be necessary, investigate more to find the real
  problem
* more thought should be given to this
@utnapischtim utnapischtim marked this pull request as ready for review November 5, 2024 09:08
@utnapischtim
Copy link
Contributor Author

NOTE: 01dd4e3 should be discussed!

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.

1 participant