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

Upgrade to Flask 2 #434

Merged
merged 1 commit into from
Mar 12, 2022
Merged

Conversation

HebaruSan
Copy link
Contributor

@HebaruSan HebaruSan commented Nov 16, 2021

Problem

After #440 was merged, alpha is no longer working (despite working fine in localhost testing).

Cause

@DasSkelett says this is caused by pallets/markupsafe#282, which says "You are using an unsupported version of Jinja," meaning that we should use the latest version of Jinja.

Background

In #353 we pinned the Flask and Jinja2 dependency versions because there are API changes that would break parts of the backend and more testing is required.

Backend logs (including deprecation warnings) can be printed with:

docker logs spacedock_backend_1 

Motivation

See python/typeshed#5423, the type stubs for Flask and Jinja2 may be removed from typeshed soon because the latest versions of those modules have type hints built-in. I'm not certain, but I think this means we might wake up one day to find that our code using the older versions no longer passes the mypy tests.

Changes

Now we install the latest versions of Flask and Jinja2, and various deprecation warnings are fixed:

  • The sqlalchemy.ext.declarative.declarative_base imports are changed to sqlalchemy.orm.declarative_base
  • The markdown.util.etree imports are changed to xml.etree.ElementTree
  • URL() is replaced by URL.create()
  • We delete the profile-dir config setting for tests so the test harness doesn't try to create dirs it can't access
  • The flask_api.status imports are changed to http.HTTPStatus
  • Since integral type hints for Jinja2 should mean that ChainableUndefined is defined properly, the PyType test harness we removed from Handle missing default mod version, don't close db before rendering templates #378 is back to provide some extra checks on our code (this is currently in flux, the test just failed as I'm typing this but I'm going to try to massage it into working after creating the PR)
    PyType still doesn't work, see comments below for details.

As far as I can tell, everything still works.

@HebaruSan HebaruSan added Type: Improvement Area: Backend Related to the Python code that runs inside gunicorn Priority: High Status: Ready Scope: Medium Moderately complex changes requiring non-trivial time and effort to develop and review labels Nov 16, 2021
@HebaruSan HebaruSan requested a review from DasSkelett November 16, 2021 20:36
@HebaruSan

This comment was marked as resolved.

@HebaruSan

This comment was marked as resolved.

@HebaruSan HebaruSan removed the request for review from DasSkelett November 16, 2021 23:56
@HebaruSan HebaruSan self-assigned this Nov 16, 2021
@HebaruSan HebaruSan force-pushed the feature/flask-2 branch 2 times, most recently from e259d47 to 0d28436 Compare March 11, 2022 18:17
@HebaruSan
Copy link
Contributor Author

PyType seems fundamentally incompatible with sqlalchemy, at least so far. It tries to interpret references to properties like Mod.description as Column, when in practice we use them as strings all throughout the code.

    if len(mod.description) < 100:
$ pytype KerbalStuff alembic/versions
Computing dependencies
Analyzing 42 sources with 0 local dependencies
ninja: Entering directory `.pytype'
[8/34] check KerbalStuff.objects
FAILED: /home/user/Downloads/KSP/SpaceDock/.pytype/pyi/KerbalStuff/objects.pyi 
/usr/bin/python3 -m pytype.single --imports_info /home/user/Downloads/KSP/SpaceDock/.pytype/imports/KerbalStuff.objects.imports --module-name KerbalStuff.objects -V 3.9 -o /home/user/Downloads/KSP/SpaceDock/.pytype/pyi/KerbalStuff/objects.pyi --analyze-annotated --nofail --quick /home/user/Downloads/KSP/SpaceDock/KerbalStuff/objects.py
File "/home/user/Downloads/KSP/SpaceDock/KerbalStuff/objects.py", line 367, in format_size: Function posixpath.join was called with the wrong arguments [wrong-arg-types]
         Expected: (__a, _1: Union[os.PathLike[str], str], ...)
  Actually passed: (__a, _1: sqlalchemy.sql.schema.Column)
  Attributes of protocol os.PathLike[str] are not implemented on sqlalchemy.sql.schema.Column: __fspath__

To proceed with adding PyType, we'll either need a smarter version of PyType or an update to sqlalchemy that somehow provides annotations of how its objects' properties can actually be used. I will now remove PyType from this PR.

@HebaruSan HebaruSan changed the title Upgrade to Flask 2, enable PyType Upgrade to Flask 2 Mar 11, 2022
@HebaruSan HebaruSan requested a review from DasSkelett March 11, 2022 18:32
Copy link
Member

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything appears to be working fine now, so let's give it a try!

@DasSkelett DasSkelett merged commit b632cb8 into KSP-SpaceDock:alpha Mar 12, 2022
@HebaruSan HebaruSan deleted the feature/flask-2 branch March 12, 2022 18:43
@HebaruSan HebaruSan mentioned this pull request Apr 25, 2022
@HebaruSan HebaruSan mentioned this pull request Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Backend Related to the Python code that runs inside gunicorn Priority: High Scope: Medium Moderately complex changes requiring non-trivial time and effort to develop and review Status: In Progress Type: Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants