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 upper limit #174

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

utnapischtim
Copy link
Contributor

@utnapischtim utnapischtim commented Aug 4, 2024

  • setup: increase flask-sqlalchemy version
  • fix: tests LocalProxy
  • change: remove click 3 compatibility
  • change: add proxy file
  • fix: WeakKeyDictionary
  • fix: remove warning and black problems
  • fix: VARCHAR needs length on mysql
  • fix: make variable names more distinguishable
  • fix: comment assert in tests_db
  • refactor: rename current_db to current_sqlalchemy
  • setup: remove upper pins
  • refactor: move MockEntryPoint to independent file
  • tests: fix sqlalchemy.exc.InvalidRequestError
  • tests: fix sqlalchemy.exc.InvalidRequestError
  • cli: password is per default hided
  • not finished commit

NOTE

worked locally with flask>=3.0.0, werkzeug>=3.0.0, flask-alemib>=3.0.0. further updated invenio-base is necessary (otherwise flask and werkzeug couldn't be updated)

ATTENTION

tests are not independent. if tests in test_db.py fail tests in test_versioning fail too although they would work without problems

DISCUSSION

some fixes should be discussed so review carfully

KNOWN issues

  • flask-sqlalchemy changes import path of pagination

* it is not working anymore with localproxy and it seams not necessary
* use the proxy file instead of creating the _db variable in both files

* this removes a DeprecationWarning from flask-sqlalchemy, which
  states that the support of the use '.db' will be removed
* the error indicates that db.init_app is not called, but it is and it
  works in other contexts. It may that the `db = invenio_db = shared.db`
  line in conftest.py is not working as expected anymore. The only found
  solution is this. The drawback is, if that is the only solution it has
  to be done also in the other packages which are using this function.
  With the default value it is backward compatible but as the tests
  indicates it may not work without adding the db parameter to the
  function call
* added D202, pydocstyle and black do not habe the same opinion

* remove warning by using StringEncryptedType instead of EncryptedType.
  This removes a warning and there is further a notice in the code that
  the base type of EncryptedType will change in the future and it is
  better to replace it with StringEncryptedType
* this change makes it possible to import the MockEntryPoints from an
  independend file. this class is used in two tests but defined in one
  of them. placing it into a separated file makes it independent.
* this leads to the problem that the program couldn't connect to the
  database
* apply new correct return values to cli calls

* refactor code

* remove unused code
* added some explanation comments

* apply db.session.query over Model.query syntax
* sqlalchemy.exc.ArgumentError: Column expression, FROM clause, or other
  columns clause element expected, got
  [<sqlalchemy.sql.elements.BinaryExpression object at 0x7fa3a0>,
  <sqlalchemy.sql.elements.BinaryExpression object at 0x7fa3ad246690>].
  Did you mean to say select(<sqlalchemy.sql.elements.BinaryExpression
  object at 0x7fa3b1145c10>, <sqly.sql.elements.BinaryExpression object at
  0x7fa3ad246690>)

  don't give the array to the select but give the elements to the select
  method

* AttributeError: 'Engine' object has no attribute 'execute' -> change
  to session

* TypeError: scoped_session.execute() got an unexpected keyword argument
  'a' -> change to dict
* the rollback should only rollback whats in that savepoint. using
  session.begin_nested creates a subtransaction which can then
  rolledback
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