Skip to content

Remove Docassemble core dependency from install_requires#13

Open
nonprofittechy wants to merge 1 commit into
networkninja:mainfrom
nonprofittechy:fix/remove-docassemble-core-dependency
Open

Remove Docassemble core dependency from install_requires#13
nonprofittechy wants to merge 1 commit into
networkninja:mainfrom
nonprofittechy:fix/remove-docassemble-core-dependency

Conversation

@nonprofittechy

Copy link
Copy Markdown
Contributor

Summary

Remove the docassemble.webapp>=1.4.54 packaging dependency from install_requires.

Why

This package is being installed into Docassemble servers that already provide the Docassemble core stack. Keeping a broad docassemble.webapp>=... dependency allows pip to resolve Docassemble core packages from PyPI during managed package installs.

In a live Docassemble-managed install test, that dependency caused the resolver to walk into older core releases from PyPI, including docassemble.base==1.7.0 and docassemble.demo==1.7.0, instead of preserving the server's installed 1.9.2 core stack.

This dependency appears to have originally been used as a minimum-version guard for older Docassemble servers, but at this point it is creating upgrade risk for current servers. Removing the core dependency lets Docassemble treat its own core packages as part of the platform rather than something this extension should ask pip to resolve.

Scope

This changes packaging metadata only. Runtime code is unchanged.

Verification

  • python3 setup.py egg_info succeeds after this change
  • managed install testing against a live Docassemble server showed the resolver problem described above

@michaelhofrichter michaelhofrichter left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The change is correct and the PR description explains the problem well. Removing docassemble.webapp>=1.4.54 from install_requires is the right fix — that's the entry point pip uses during package resolution, and having it there was causing the resolver to pull in older core releases from PyPI on live servers.

Two small things to consider before merging:

  1. requirements.txt still has the same entriesdocassemble/LegalServerLink/requirements.txt still lists both docassemble.base>=1.4.54 and docassemble.webapp>=1.4.54. If CI or any dev tooling runs pip install -r requirements.txt, the resolver problem could still appear there. Since that file also includes mypy and lxml (which aren't in install_requires), it's clearly a dev/CI file rather than a production one — but it's worth deciding whether to strip the core deps there too for consistency.

  2. No version bumpsetup.py still says 1.2.7. This is a packaging-only change so the maintainers may handle versioning separately, but if this is published to PyPI as-is, existing installs won't pick it up on a standard upgrade. A patch bump (e.g. 1.2.8) would make the fix discoverable via pip.

Comment thread setup.py
"requests>=2.31.0",
"pycountry>=22.3.5",
"docassemble.webapp>=1.4.54",
"defusedxml>=0.7.1",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The removal here is correct and well-reasoned. install_requires is what pip uses during package resolution, so keeping docassemble.webapp there was the root cause of the resolver walking into older PyPI releases of the core stack.

One thing to consider: docassemble/LegalServerLink/requirements.txt still lists both docassemble.base>=1.4.54 and docassemble.webapp>=1.4.54. If that file is used in CI or local dev (pip install -r requirements.txt), the same resolver issue could surface there. It looks like a dev/CI file (it also includes mypy and lxml which aren't in install_requires), so it may be intentionally separate — but worth confirming whether it should get the same treatment for consistency.

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.

3 participants