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

Revert pyparsing constraint #482

Merged
merged 2 commits into from Nov 2, 2021
Merged

Conversation

moyiz
Copy link
Contributor

@moyiz moyiz commented Nov 2, 2021

See:
#471
#480

This PR removes the upper limit on pyparsing version and fixes the unit test to accept the exception message of either version 2 or 3. This seem to be the minimal change needed to support both major versions for now (until the camel case synonyms will be removed).

See: https://github.com/pypa/packaging/pull/480/files#r740158146

As suggested by @layday, local `packaging` package directory takes
precedence over the installed `packaging`. By building with
`pyproject-build`, CWD will not be added to python path.
See discussion in : pypa#480
`pyparsing` is backward compatible for now (synonyms for camel cased
attributes).
Fix unit test to accecpt either pyparsing 2 or 3 exception message.
@pradyunsg pradyunsg merged commit 8cb9dbf into pypa:main Nov 2, 2021
@pradyunsg
Copy link
Member

Thanks @moyiz for the PR, and @henryiii for the discussion around this!

@EwoutH
Copy link

EwoutH commented Nov 8, 2021

Awesome, thanks for this PR! Could a new release be tagged with this fix? That would help a ton solving dependency conflicts.

@jeroenbbb
Copy link

Google app engine deployment fails because of the upper limit so I expect many are facing an issue; a new release would be much appreciated...

@henryiii
Copy link
Contributor

henryiii commented Nov 8, 2021

I'm going to be releasing a blog post roasting the practice of limiting upper versions constraints in the future; any extra ammo you have from experience would be helpful. :)

Also, a new release is not really needed, the current release just needs to be yanked. It wasn't capped before.

@layday
Copy link
Member

layday commented Nov 8, 2021

The version cap inadvertently prevented breakage on a much larger scale. The latest release of pyparsing removed two private attributes that packaging is using.

@henryiii
Copy link
Contributor

henryiii commented Nov 8, 2021

What two attributes? Can't find the PR with a quick search.

@layday
Copy link
Member

layday commented Nov 8, 2021

Please see #483 (comment).

@henryiii
Copy link
Contributor

henryiii commented Nov 8, 2021

Ah, just found it. That was an update from 3.0.4 to 3.0.5, though; so the correct version constraint would have been <3.0.5 or !=3.0.5 😂. We should not have been using private variables, or we should use our own parser (seems like there's already a PR for that ;) )

@henryiii
Copy link
Contributor

henryiii commented Nov 8, 2021

Also, with the version constraint, that breakage in 3.0.5 would not have been discovered so quickly.

@jeroenbbb
Copy link

jeroenbbb commented Nov 9, 2021 via email

htuch pushed a commit to envoyproxy/envoy that referenced this pull request Nov 9, 2021
This reverts the bump to pyparsing in PRs:

build(deps): bump pyparsing from 3.0.4 to 3.0.5 in /tools/dependency #18937
dependabot: Python updates #18929
the pyparsing version is not compatible with the current version of packaging

the issue is fixed upstream pypa/packaging#482 but the fix is not yet included in any release

this issue is currently causing the github deps checker action to fail

Signed-off-by: Ryan Northey <ryan@synca.io>
probberechts added a commit to ML-KULeuven/socceraction that referenced this pull request Nov 11, 2021
nox-poetry crashed in the GA workflow when parsing requirements with packaging < 21.1 and pyparsing >= 3

See pypa/packaging#482 for the upstream discussion.
@henryiii
Copy link
Contributor

Looks like this was fixed in pyparsing 3.0.6. So the correct constraint was !=3.0.5. :)

@johnthagen
Copy link
Contributor

To echo @EwoutH's comment, could a new release be tagged? This would help dependency conflicts (for example, when using pip-tools and multiple requirements.txt files).

@brettcannon
Copy link
Member

@johnthagen see #483 for cutting another release.

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.

None yet

8 participants