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

Implement requests pool #502

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

atosz33
Copy link

@atosz33 atosz33 commented Jun 22, 2021

Wordpress/Requests currently lacking of requests pool, which instead of multiple requests, use a max size of simultaneous requests in one shot (pool_size), but allows more requests to handle.

Kis Attila Balint added 3 commits June 23, 2021 14:47
@atosz33 atosz33 force-pushed the requests-pooling branch 2 times, most recently from f7e8cda to 54e4921 Compare June 29, 2021 14:51
@andras-tim
Copy link

Hi, Is there any news on this project?
May I ask for some review on that to be able to be tested as well?

Copy link

@andras-tim andras-tim left a comment

Choose a reason for hiding this comment

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

I tested this change, and it looks like works + the formatting is also good as I think

@andras-tim
Copy link

@jrfnl what's the next step?

May I ask to merge this feature into the library?

@jrfnl
Copy link
Member

jrfnl commented Jul 5, 2021

@andras-tim @atosz33 Thank you both for your interest in and willingness to contribute to Requests.

For your information:

This repo has had little maintenance over the past few years and had build up quite some technical debt. We're currently working on some structural changes - largely outlined in #460 - to improve the basic setup and remove/diminish some of the technical debt to make the library more easily maintainable for the future.

Once those initial changes have been made, we'll start doing (public) triage sessions, going through the open issues and open PRs (also outlined in #460).

This ticket will be looked at during those sessions and I'd very much would like to invite you to join these sessions.
The triage sessions will be announced in ticket #460, so if you subscribe to that ticket you will be notified when the first session is announced.

As for the specific changes in this ticket: I can see the changes are accompanied by tests, but the tests do not (yet) fully cover the change, which can also be seen in the codecov patch report. Making sure that the tests are comprehensive enough to fully cover the change will help get this PR merged more quickly.

Hope this helps clarify where we are at this moment.

@andras-tim
Copy link

Dear @jrfnl,

Thank you for the detailed information. Of course, I'm open to some round of any kind of sessions - if I can help. 🙂

We are using your lib for years - besides that, many thanks - and now we had a sprint there this pool-based parallel behavior was missed. Therefore @atosz33 has forked, implemented, and pushed back the change.

So, the team where I'm working will try to spend time on this PR but I will be glad if the merging process will not take months.

Thank you for your work again.

@jrfnl
Copy link
Member

jrfnl commented Aug 13, 2021

Hiya!

We're currently planning two triage sessions for Requests 2.0 and would like to invite you to join us in one or both of these sessions to talk us through the PR and discuss it.

The triage sessions are currently planned for:

  • Friday August 20, 07:00 - 11:00 UTC
  • Friday September 3, 07:00 - 11:00 UTC

Would you be available to join us during those times on one of those days ?

@andras-tim
Copy link

Hiya!

We're currently planning two triage sessions for Requests 2.0 and would like to invite you to join us in one or both of these sessions to talk us through the PR and discuss it.

The triage sessions are currently planned for:

  • Friday August 20, 07:00 - 11:00 UTC
  • Friday September 3, 07:00 - 11:00 UTC

Would you be available to join us during those times on one of those days ?

Dear @jrfnl,
the 20th of August is a national holiday in Hungary, therefore this isn't the best for me and my team.
So I would like to discuss things on the 3rd of September.
Thx!

@jrfnl
Copy link
Member

jrfnl commented Aug 19, 2021

@andras-tim Wishing you a good national holiday and I look forward to talking with on Sept 3rd. Got a preference for a time within the above mentioned time slot ?
We'll be using Zoom for these triage sessions and we'll make sure you have access to the link before the session.

@andras-tim
Copy link

andras-tim commented Aug 20, 2021

@andras-tim Wishing you a good national holiday and I look forward to talking with on Sept 3rd. Got a preference for a time within the above mentioned time slot ?
We'll be using Zoom for these triage sessions and we'll make sure you have access to the link before the session.

Thanks!
I prefer the 8:00 - 9:00 UTC, but the whole range can be acceptable for an hour.

@jrfnl
Copy link
Member

jrfnl commented Aug 30, 2021

@andras-tim Thanks for making the time. 8:00 UTC will be fine. I look forward to speaking with you this Friday!

@jrfnl
Copy link
Member

jrfnl commented Sep 2, 2021

Here's the link to tomorrow's video call: https://us02web.zoom.us/j/87276335771?pwd=dFNXbjVxcWtuM213TUlVejMvQzVXdz09

Looking forward to it!

@jrfnl
Copy link
Member

jrfnl commented Sep 16, 2021

FYI: link to tomorrow's triage session if you want to continue the conversation: https://us02web.zoom.us/j/85765782668?pwd=Z1IrUFFaR2lhZFIvcDhCeGJrc1JHdz09

@schlessera schlessera added this to the 2.1.0 milestone Nov 5, 2021
Copy link

@LeHoang88 LeHoang88 left a comment

Choose a reason for hiding this comment

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

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants