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 python-engineio #8099

Closed
wants to merge 8 commits into from
Closed

Bump python-engineio #8099

wants to merge 8 commits into from

Conversation

pheel
Copy link
Contributor

@pheel pheel commented Mar 3, 2021

Fixes #8092

Status (please check what you already did):

  • updated the changelog (please check changelog for instructions)

@sara-tagger sara-tagger requested a review from b-quachtran March 4, 2021 07:00
@sara-tagger
Copy link
Collaborator

Thanks for submitting a pull request 🚀 @b-quachtran will take a look at it as soon as possible ✨

@b-quachtran b-quachtran requested review from alwx and removed request for b-quachtran March 9, 2021 00:41
@alwx alwx requested review from joejuzl and removed request for alwx March 11, 2021 10:25
@@ -101,8 +101,8 @@ multidict = "^4.6"
aiohttp = "~3.6"
questionary = "~1.5.1"
prompt-toolkit = "^2.0"
python-socketio = ">=4.4,<6"
python-engineio = ">=3.11,<3.14"
python-socketio = ">=5,<6"
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a specific reason we need to change the python-socketio version range?

python-socketio = ">=4.4,<6"
python-engineio = ">=3.11,<3.14"
python-socketio = ">=5,<6"
python-engineio = ">=4,<5"
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a specific reason we have to increase the minimum bound?

Copy link
Contributor Author

@pheel pheel Mar 11, 2021

Choose a reason for hiding this comment

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

Because of >=4.4,<6, python-socketio accidentally got bumped to 5, which is incompatible with python-engineio >=3.11,<3.14. python-socketio and python-engineio need to be in lockstep, as explained by rgstephens here: botfront/rasa-webchat#344

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, but can't poetry handle finding compatible versions if there exists a valid overlap?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would have been the hope, but there's no denying something went wrong here.

Copy link
Contributor Author

@pheel pheel Mar 17, 2021

Choose a reason for hiding this comment

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

python-socketio 5.0.0 had bad deps, that only got corrected in 5.0.1.. This means the current pyproject.toml will always resolve to this incompatible situation (python-socketio 5.0.0 with python-engineio 3.13.2). Either you bump (like I did), or you downgrade.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok I see 👍
Would it be possible to just exclude python-socketio 5.0.0?
We would like to be able to keep a wider range.

@tmbo
Copy link
Member

tmbo commented Apr 13, 2021

fixed as part of #8343

@tmbo tmbo closed this Apr 13, 2021
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.

python-socketio and python-engineio versions are incompatible
5 participants