@nateprewitt please take a look.

This has been open roughly a day. Please do not ping maintainers directly like this

This has been open roughly a day. Please do not ping maintainers directly like this

My apologies. The issue this PR tries to fix #6735 has been open for a while and we have pinged on that issue several times with no engagement for a fix. Hence, I wanted to escalate. Thanks.

You're escalating as a salaried employee of a company who invest in this to two people who aren't paid to work on this project

You're escalating as a salaried employee of a company who invest in this to two people who aren't paid to work on this project

Please understand that we are not trying to add stress to anyone. Its not personal. We have tried to engage on the GH issue that has been open for almost one year. There was no resolution. We are being pushed by our customers for a fix. We investigated the issue on our end, proposed a few solutions on that issue. There was no response from maintainers. Therefore, I had to send this MR and draw their attention. If I had the merge rights I would have simply merged this myself since I see the change as a low risk item.

You're escalating as a salaried employee of a company who invest in this to two people who aren't paid to work on this project

If I had the merge rights I would have simply merged this myself since I see the change as a low risk item.

@ani-sinha Are you calling peer reviews pointless because you know best and can anticipate all the implications this change might have?
Don't get me wrong ... I worked on the Cloudinit / OpenStack issue (canonical/cloud-init#5419) and would love for this to work with every distribution including IBM.

But requests is a library: ANY change there must receive the proper review - no drive-by fixes.

@ani-sinha Are you calling peer reviews pointless because you know best and can anticipate all the implications this change might have?

I never said that and please refrain from drawing such conclusions.

But requests is a library: ANY change there must receive the proper review - no drive-by fixes.

yes of course! I said, it was my opinion that the change is safe enough. Therefore, if I had the merge rights, I would merge them without bothering the maintainers provided there were no objections (seems so far I have seen no technical arguments against this change only concern that I am bothering unpaid maintainers too early). If people have technical objections against this change they can certainly raise it here. Which is the whole point of raising this PR. Also note that even when a change it merged, if someone raises serious objections, it can also be reverted later. BUT, we need to make progress which is what has been lacking on this issue so far. Our customers are frustrated and pushing us. We need to provide them with answers.

@ani-sinha Are you calling peer reviews pointless because you know best and can anticipate all the implications this change might have?

I never said that and please refrain from drawing such conclusions.

But requests is a library: ANY change there must receive the proper review - no drive-by fixes.

yes of course! I said, it was my opinion that the change is safe enough. Therefore, if I had the merge rights, I would merge them without bothering the maintainers provided there were no objections (seems so far I have seen no technical arguments against this change only concern that I am bothering unpaid maintainers too early). If people have technical objections against this change they can certainly raise it here. Which is the whole point of raising this PR. Also note that even when a change it merged, if someone raises serious objections, it can also be reverted later. BUT, we need to make progress which is what has been lacking on this issue so far. Our customers are frustrated and pushing us. We need to provide them with answers.

We are all professionals here - no response does not imply there are not objections. "LGTM" is the VERY LEAST you'd have to see to even come to your conclusion that your proposed change looks safe to someone else.

While I totally agree that fixing an issue at the root (the library in this case) is the right, while slower, choice!
Maybe you might be able to provide / write up why and how requests broke along the way. If I understood you right this is somewhat of a regression with "just" >=2.32.3 ? Is the issue related to c0813a2 then?

We are all professionals here - no response does not imply there are not objections. "LGTM" is the VERY LEAST you'd have to see to even come to your conclusion that your proposed change looks safe to someone else.

It varies from community to community. Sometimes in the lack of such explicit approval it is implicitly assumed that there are no strong objection from anyone on the change and its safe to go in. Its a judgement call on the part of the maintainer (who btw, sometimes may not also know all the details or familiar with that particular area of the code).

why and how requests broke along the way

The discussion in the issue has the details. I have added the reference to the issue in the commit log.
TL; DR:
The change that broke requests is c0813a2 . It uses urlparse() which is not able to handle link local ipv6 addresses with port names. When you added canonical/cloud-init@af4ac7f to cloud-init, it started breaking because of this.
The suggestion in this change is to use parse_url() from utllib3 instead. This is already used elewhere in the code and the library is also already imported. So impact is minimal and the change has been tested to fix the issue and not cause any apparent regression.

no response does not imply there are not objections. "LGTM" is the VERY LEAST you'd have to see to even come to your conclusion that your proposed change looks safe to someone else.

Also from practical point of view, a critical fix that addresses a regression cannot wait in the queue indefinitely until someone says LGTM. When a fair amount of time has passed (which in my option it has for this issue), a call needs to be made.

no response does not imply there are not objections. "LGTM" is the VERY LEAST you'd have to see to even come to your conclusion that your proposed change looks safe to someone else.

Also from practical point of view, a critical fix that introduces regression cannot wait in the queue indefinitely until someone says LGTM. When a fair amount of time has passed (which in my option it has for this issue), a call needs to be made.

You meant to say "that fixes a regression" right - I totally agree. A bug marked as or containing a regression should be higher priority than other issues. But in the end ... no matter how well you organize your work, if there is too much it will overwhelm you anyways.

Enough of the off-topic talk - thanks for staying at this issue and proposing a fix.

Also from practical point of view, a critical fix that addresses a regression cannot wait in the queue indefinitely until someone says LGTM.

The confidence you have in this is astounding. With the context that Nate and I have we know that every url parsing change is a minefield. Each parser, regardless of how standard compliant (or browser behavior compliant) breaks someone and is a regression for someone

The confidence you have in this is astounding

The fact that we are having endless useless debate without actually making any progress - that is quite astounding to me.
We have done basic testing. We have found no apparent regression. If we can wait 10 years and try all kinds of new exhaustive ways to test this, no problem wth me. I will ask our team to apply this change downstream and move on.

To be frank, I don't even care whether this change gets merged or not. I want the issue to be fixed in whichever way the standards of this community are met. No one was actually taking any interest in sending a PR for a fix for the issue. I did. Now I almost regret it.

Thanks for your inputs. I'm done. It will be the last time you'll hear from me on this issue.