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 18 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
* the problem is, that assert calls an function which leads to an not
  moving forward test on mysql8. it works on postgresql and mysql5 but not
  mysql8.
* the new name better reflects the intended use
* 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.
* the complete error message is "Table 'early_class' is already defined
  for this MetaData instance. Specify 'extend_existing=True' to redefine
  options and columns on an existing Table object."

  adding the extend_existing=True doesn't solve the problem. The error
  was there because of the reuse introduced by the
  @pytest.mark.parametrize decorator.
* the complete error message is "Table 'versioned_article_b_version' is
  already defined for this MetaData instance. Specify
  'extend_existing=True' to redefine options and columns on an existing
  Table object."

  adding the extend_existing=True doesn't solve the problem. moving the
  import of the models to the top of the file did it
* this leads to the problem that the program couldn't connect to the
  database
* for explanations see the comments in the code
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