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.

Sign up for GitHub

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

Jump to bottom

Do not reuse requests sessions #3230

Merged
merged 1 commit into from
Oct 23, 2020
Merged

Do not reuse requests sessions #3230

merged 1 commit into from
Oct 23, 2020

Conversation

abn
Copy link
Member

@abn abn commented Oct 17, 2020

When running under certain environment, reuse of request sessions maybe
causing network connectivity issues. This change ensures that request
sessions are not reused across requests.

Relates-to: #3219

@abn
Do not reuse requests sessions
4917d7b 
When running under certain environment, reuse of request sessions maybe
causing network connectivity issues. This change ensures that request
sessions are not reused across requests.

Relates-to: python-poetry#3219
@abn abn requested a review from a team October 17, 2020 01:14
@abn abn mentioned this pull request Oct 17, 2020
Closed
3 tasks
@abn abn marked this pull request as ready for review October 20, 2020 21:36
@abn abn added the kind/bug Something isn't working as expected label Oct 20, 2020
@abn abn added this to the 1.1 milestone Oct 20, 2020
@abn
Copy link
Member Author

abn commented Oct 20, 2020

This seems to fix the issue; verified by the community on the issue.

@sloev
Copy link

sloev commented Oct 21, 2020

please get this released asap, we have had broken Travis builds for many days now because of this.

@abn
Copy link
Member Author

abn commented Oct 21, 2020

@sloev I can appreciate your urgency. But please consider that this is a community project with a very small team of volunteers. Would appreciate it if you refrained from demands, I am going to assume this was not the intention.

As for your build issues. You can consider temporarily disabling the experimental installer as described here untill 1.1.4 is released.

@sloev
Copy link

sloev commented Oct 22, 2020

@abn it wasn't my intention, sorry if it came across as such. (personal lesson learned: dont communicate on gh issues when under pressure)
I will join the discord and see if i can help out instead of these "bursts of frustration"
Have a great day everybody and sorry for the noise 🤗

sdispater
sdispater approved these changes Oct 23, 2020
View reviewed changes
Copy link
Member

@sdispater sdispater left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

However, do you know whether the overhead of having to create new sessions for each download is negligible or not?

@abn
Copy link
Member Author

abn commented Oct 23, 2020

However, do you know whether the overhead of having to create new sessions for each download is negligible or not?

There will be a performance hit. Last I checked, for around 50 packages in a clean environment it was around 2-3 seconds. That said, it will vary according to groups etc. But I think it is reasonable enough for now until we move to a more ... parallel safe http client. Right now since this is a major issue in container environments, I think we should go ahead with it.

@abn abn merged commit bf17dde into python-poetry:1.1 Oct 23, 2020
@abn abn deleted the issue/3219 branch October 23, 2020 09:54
@liuzhenqi77 liuzhenqi77 mentioned this pull request Nov 9, 2021
Open
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 1, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sdispater sdispater sdispater approved these changes

Assignees
No one assigned
Labels
kind/bug Something isn't working as expected
Projects
None yet
Milestone
1.1
Development

Successfully merging this pull request may close these issues.
3 participants
@abn @sloev @sdispater