Skip to content

Conversation

@mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Mar 15, 2022

We already do that in practice:

vendor/golang.org/x/net/http2/transport.go:417:45: undefined: os.ErrDeadlineExceeded

so make that official to possibly benefit from that.

Then adopt various newly available API; see the individual commits for details.

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM
Nice work!

@vrothberg
Copy link
Member

@mtrmac could you rebase just to be sure everything's in order?

@mtrmac mtrmac force-pushed the go-1.15 branch 3 times, most recently from aa91955 to cfc719b Compare March 16, 2022 15:10
@mtrmac
Copy link
Collaborator Author

mtrmac commented Mar 16, 2022

Rebased, tests pass.

@TomSweeneyRedHat
Copy link
Member

LGTM
very nicely done @mtrmac

@TomSweeneyRedHat
Copy link
Member

/lgtm

mtrmac added 6 commits March 17, 2022 20:35
Signed-off-by: Miloslav Trmač <[email protected]>
…ccept an *url.URL

This is, sadly, wasteful, because NewRequestWithContext() only accepts
a string and parses it again, but it gives us more type safety, and simplifies
at least some callers.

Most importantly, this will also allow us to call url.Redacted() for logging.

Signed-off-by: Miloslav Trmač <[email protected]>
... to be at least a bit protected against credentials in logs.

I did try to find all uses, but it's possible I have missed some.

Signed-off-by: Miloslav Trmač <[email protected]>
... to simplify tests.

Signed-off-by: Miloslav Trmač <[email protected]>
We no longer need a cleanup callback.

Signed-off-by: Miloslav Trmač <[email protected]>
@rhatdan rhatdan merged commit afdffaa into containers:main Mar 19, 2022
@mtrmac mtrmac deleted the go-1.15 branch March 19, 2022 18:34
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.

4 participants