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

Bump dnspython to 2.6.1 to fix pack CI failure on py3.10/py3.11 #6265

Merged
merged 2 commits into from
Oct 24, 2024

Conversation

nzlosh
Copy link
Contributor

@nzlosh nzlosh commented Oct 19, 2024

Update dnspython to v2.6.1 to fix pack CI failing for py3.10 / py3.11 with AttributeError: module 'collections' has no attribute 'MutableMapping'.

@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Oct 19, 2024
@nzlosh nzlosh enabled auto-merge (squash) October 19, 2024 08:50
@nzlosh nzlosh added this to the 3.9.0 milestone Oct 19, 2024
@nzlosh nzlosh requested a review from cognifloyd October 19, 2024 08:54
@cognifloyd
Copy link
Member

dnspython is a transitive dep -- we don't use it directly. It looks like it is a dependency of:

  • eventlet: dnspython>=1.15.0
  • pymongo: dnspython<3.0.0,>=1.16.0

What if we just removed dnspython from our requirements files and let pip pick the version?

Or, what if we just removed the constraint from lockfiles/st2-constraints.txt and update fixed-requirements.txt with whatever version got locked (this is probably similar to what you've done here)?

@cognifloyd
Copy link
Member

To clarify: the original reason for pinning dnspython in #4997 (an incompatibility between dnspython and pymongo) no longer applies. We've upgraded pymongo several times since then, and it clearly supports newer versions, so the constraint is not needed any more.

@nzlosh
Copy link
Contributor Author

nzlosh commented Oct 19, 2024

Dropped pinning for dnspython as suggested.

Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

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

Looks like the lockfile was regenerated with these requirements changes (which includes transitive deps). Normally regenerating the lockfile displays a summary of these changes--I like to copy that into a comment on the PR.

  • arcomplete 3.5.0 -> 3.5.1
  • charset-normalizer 3.3.2 -> 3.4.0
  • cryptography 43.0.1 -> 43.0.3
  • distlib 0.3.8 -> 0.3.9
  • dnspython 1.16.0 -> 2.6.1
  • greenlet 3.1.0 -> 3.1.1
  • prompt-toolkit 3.0.47 -> 3.0.48
  • psutil 6.0.0 -> 6.1.0
  • redis 5.0.8 -> 5.1.1
  • setuptools 75.1.0 -> 75.2.0
  • tomli 2.0.1 -> 2.0.2
  • tzdata 2024.1 -> 2024.2
  • virtualenv 20.26.5 -> 20.27.0
  • xmltodict 0.13.0 -> 0.14.2

Would you please update these in fixed-requirements.txt to match the lockfile?

  • argcomplete
  • cryptography
  • greenlet
  • prompt-toolkit
  • psutil
  • redis
  • virtualenv

And please update this in the Makefile:

  • setuptools

Aside: eventually, pants should gain the ability to update only some of the deps instead of regenerating the whole thing. Sorry for the mess in the interim.

If you'd prefer, we can submit these extra fixed-requirements.txt changes in a follow-up PR. Because everything else LGTM. I'm approving this for now, assuming these updates are made in such a follow-up.

@nzlosh nzlosh requested a review from a team October 20, 2024 07:43
@pull-request-size pull-request-size bot added size/XL PR that changes 500-999 lines. Consider splitting work into several ones that easier to review. and removed size/L PR that changes 100-499 lines. Requires some effort to review. labels Oct 20, 2024
@nzlosh nzlosh requested review from mamercad and guzzijones October 21, 2024 14:27
@nzlosh nzlosh merged commit 2271db0 into StackStorm:master Oct 24, 2024
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XL PR that changes 500-999 lines. Consider splitting work into several ones that easier to review.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants