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.

Sign up for GitHub

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

Jump to bottom

Dependencies via pip extras #2476

Merged
merged 9 commits into from
Jul 25, 2024
Merged

Dependencies via pip extras #2476

merged 9 commits into from
Jul 25, 2024

Conversation

samuelhwilliams
Copy link
Contributor

@samuelhwilliams samuelhwilliams commented Jul 24, 2024 •
edited
Loading

We previously haven't ever had a single place that clearly says what dependencies Flask-Admin expects for the 'optional' features, and what versions of those dependencies are required.

This PR pulls all of this information into pyproject.toml and exposes them as extras.

All of the examples have been updated to install using these extras, which simplifies their requirements files.

OLD NEWS

temporarily disabled `constrain_package_deps`

**This is now outdated and a recent commit on the PR has re-enabled this by adding a constraints file.**

The controversial change here is that, now, in our 'standard' test runs, we can't get the flask-mongoengine tests to pass because that dependency is very out of date and doesn't support the latest versions of Flask that we support. We may need to consider deprecating/removing flask-mongoengine altogether, but I'm leaving that decision to be had on this thread: #2466. I've added a 'snowflake' tox env for flask-mongoengine that pins required (old) versions of flask/werkzeug/sqlalchemy in the mean time and disabled flask-mongoengine tests in the main suites.

I've also had to disable constrain_package_deps in tox.ini as we're getting a dependency conflict from google's appengine-python-standard, which requires urllib3<2. I've raised an issue on this here: GoogleCloudPlatform/appengine-python-standard#121

The conflict is caused by:
    appengine-python-standard 1.1.6 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 1.1.5 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 1.1.4 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 1.1.3 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 1.1.2 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 1.1.1 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 1.1.0 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 1.0.0 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 0.3.1 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 0.3.0 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 0.2.4 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 0.2.3 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 0.2.2 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 0.2.1 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 0.2.0 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 0.1.1 depends on urllib3<2 and >=1.26.2
    appengine-python-standard 0.1.0 depends on urllib3<2 and >=1.26.2
    The user requested (constraint) urllib3==2.2.2

To fix this you could try to:
1. loosen the range of package versions you've specified
2. remove package versions to allow pip to attempt to solve the dependency conflict

ERROR: Cannot install appengine-python-standard==0.1.0, appengine-python-standard==0.1.1, appengine-python-standard==0.2.0, appengine-python-standard==0.2.1, appengine-python-standard==0.2.2, appengine-python-standard==0.2.3, appengine-python-standard==0.2.4, appengine-python-standard==0.3.0, appengine-python-standard==0.3.1, appengine-python-standard==1.0.0, appengine-python-standard==1.1.0, appengine-python-standard==1.1.1, appengine-python-standard==1.1.2, appengine-python-standard==1.1.3, appengine-python-standard==1.1.4, appengine-python-standard==1.1.5 and appengine-python-standard==1.1.6 because these package versions have conflicting dependencies.
ERROR: ResolutionImpossible: for help visit https://pip.pypa.io/en/latest/topics/dependency-resolution/#dealing-with-dependency-conflicts

Documentation update

image

fixes: #2452

@samuelhwilliams
Record our dependencies using pip extras
6cad6c1 
Flask-Admin does not yet support Flask v3+, WTForms v3+, or SQLAlchemy
2+. Let's clearly mark this.

And then we also define a set of pip extras, which should let users pull
in the supported versions of dependencies for additional features (eg
supporting geography, mapping, redis CLI, exporting to lots of formats,
etc).

And fix our minimum python version from 3.6 to 3.8. This was done a
little while ago, but not updated here correctly.
@samuelhwilliams
Update tox dependencies/setup
aa6d644 
And pin requirements files again now that we don't include any direct
project dependencies within the `requirements/*` files.
@samuelhwilliams
Address test failures after changing how we specify dependencies
69bb9ac 
- Disable `mongoengine` tests. This is the main problem here:
  `Flask-Mongoengine` is now fairly out-of-date and doesn't support
  recent Flask 2.x versions, which our tests try to pull in. With the
  change to how we declare our dependencies (flask), we can't really
  install a specific version of flask that's lower than what we support
  across all of Flask-Admin, which we'd need to do. I don't think we
  want a special case for Flask-Mongoengine. I think we need to accept
  that Flask-Mongoengine is now unsupported and doesn't work with
  Flask-Admin, assuming people are using recent versions of Flask etc.
  We may need to consider deprecating Flask-Mongoengine support
  altogether, but I'm leaving that for a separate issue.

- flask.Markup was removed; use markupsafe instead

- Use BytesIO for file uploads

        rv = client.post('/admin/myfileadmin/upload/',
>                        data=dict(upload=(StringIO(""), 'dummy.txt')))
E       TypeError: a bytes-like object is required, not 'str'

- Update import path

- Ignore deprecation warning from flask.testing

- Flask now requires all routes/views to be registered before the first request

- Werkzeug 2.3.0+ no longer quotes URL params

- Ignore SQLAlchemy2 deprecation warning
@samuelhwilliams
Add a snowflake test flow for flaskmongoengine
033b698
@samuelhwilliams samuelhwilliams mentioned this pull request Jul 24, 2024
Open
18 tasks
@samuelhwilliams samuelhwilliams mentioned this pull request Jul 24, 2024
Closed
@samuelhwilliams samuelhwilliams changed the title Depedencies via pip extras Dependencies via pip extras Jul 25, 2024
@samuelhwilliams
Update import error messages to suggest installing extras
ee20ab9
@samuelhwilliams
Add tests-constraints.txt
dba4c21 
appengine-python-standard is becoming slightly out-of-date; it requires
urllib3 < 2.0.0.

Our tests dependencies want to install a version above 2.0.0, and with
`constrain_package_deps` enabled this then causes tox to be unable to
install flask-admin's dependencies because they conflict with the test
dependencies.

Using a constraints file we can force tox to install urllib3<2 when
installing test deps, which then allows flask-admin to be installed with
no dependency conflicts.
@hasansezertasan
Copy link
Member

hasansezertasan commented Jul 25, 2024 •
edited
Loading

I'm pleased that you renamed some dependencies. I hated the db- prefix... 👏

What about using Hatch 🚀 to manage project environments and dependencies? This way, we would get rid of requirements folder.

@samuelhwilliams
Copy link
Contributor Author

I'm pleased that you renamed some dependencies. I hated the db- prefix... 👏

So did I, about 5 minutes after doing it 😂 It does feel better without.

I think re: using hatch - there's probably quite a few options at the moment (poetry, hatch, rye, etc) - and I think as part of the pallets ecosystem we probably need to stay in lock-step with what the rest of the projects are doing generally. So I'm not super keen to visit that yet, but maybe worth kicking off a discussion in the discord?

@hasansezertasan
Copy link
Member

So did I, about 5 minutes after doing it 😂 It does feel better without.

😆

I think re: using hatch - there's probably quite a few options at the moment (poetry, hatch, rye, etc)

Did we ever use Hatch with Flask Admin before? Poetry is another great option but it doesn't support independent environments...

and I think as part of the pallets ecosystem we probably need to stay in lock-step with what the rest of the projects are doing generally. So I'm not super keen to visit that yet, but maybe worth kicking off a discussion in the discord?

Ehem... let's do that 😄.

@cjmayo
Copy link
Contributor

cjmayo commented Jul 25, 2024

I think this will make things a lot clearer for people, and lower maintenance by not individually listing the requirements for each of the examples.

I can't say I've had the time to review every line, but go for it - I do see a number of fixes for newer dependencies in here.

An alternative tests-constraints.txt would be the way to do minimum requirements?

@samuelhwilliams
Copy link
Contributor Author

@cjmayo I believe so, yes - following what flask-sqlalchemy does:

https://github.com/pallets-eco/flask-sqlalchemy/blob/main/tox.ini#L15-L17
https://github.com/pallets-eco/flask-sqlalchemy/blob/main/requirements-skip/tests-min.in

@samuelhwilliams samuelhwilliams merged commit 50bfe0a into master Jul 25, 2024
9 checks passed
@samuelhwilliams samuelhwilliams deleted the depedencies-via-pip-extras branch July 25, 2024 21:52
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use extras to define packaging options and their dependencies
3 participants
@samuelhwilliams @hasansezertasan @cjmayo