Conversation
There was a problem hiding this comment.
Pull request overview
This PR upgrades the project’s Django baseline and refreshes related Python dependencies, while updating thread-condition notifications to the modern notify_all() API.
Changes:
- Bump Django from 4.2.27 to 5.2.11 in Poetry dependencies and regenerate
poetry.lock. - Upgrade several runtime and dev dependencies (e.g.,
fasteners,filelock,lxml,xmltodict,pytest-django,pytest-mock). - Replace deprecated
notifyAll()withnotify_all()onthreading.Conditioninstances.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| requirements-dev.txt | Updates pytest-related dev dependencies. |
| pyproject.toml | Pins Django 5.2.11 and bumps several dependency version constraints. |
| poetry.lock | Regenerated lockfile reflecting the upgraded dependency graph. |
| impl/state.py | Uses Condition.notify_all() instead of deprecated notifyAll(). |
| impl/ezid.py | Uses Condition.notify_all() for lock release and unpause notifications. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sfisher
left a comment
There was a problem hiding this comment.
Hi Jing,
I spent a lot of time reconfiguring my python environment and updating before I could get this working. I also had some homebrew and mysql version problems (needed 8.4 for the connection settings with our dev database).
Anyway, once I got it all working it seems ok. I did notice that in the FAQ this looks a bit weird (with big spaces after some items), but that is quite possibly a pre-existing problem probably very low priority and maybe not new to this update. (Not sure.)
Seems working well to me when I tried it out on my local and also on the dev server.
|
@sfisher Thank you Scott for reviewing and testing. Can you take a look at FAQ on EZID Prd https://ezid.cdlib.org/learn/doi_services_faq? |
|
Verified that the big spaces after some items in FAQ page is a pre-existing problem. Recorded the issue in a ticket #997, fix the issue in future UI related updates. |
@sfisher Hi Scott,
Here are the changes:
notifyAll()function withnotify_all()Please review and let me know if you have questions.
Thank you
Jing