Skip to content

Conversation

@saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented Jun 13, 2022

What type of PR is this?

/kind bug

What this PR does / why we need it:

The httpcache transport is breaking authentication where no fix seems to
be implemented yet. I'd propose to revert the change for now to be able
to use authenticated requests in tools like
k/release/cmd/release-notes.

Refers to: google/go-github#1503, gregjones/httpcache#104

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Reverts #58

Does this PR introduce a user-facing change?

Fixed broken authorization caused by httpcache.

The httpcache transport is breaking authentication where no fix seems to
be implemented yet. I'd propose to revert the change for now to be able
to use authenticated requests in tools like
`k/release/cmd/release-notes`.

Refers to: google/go-github#1503,
gregjones/httpcache#104

Signed-off-by: Sascha Grunert <[email protected]>
@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 13, 2022
@k8s-ci-robot k8s-ci-robot requested a review from onlydole June 13, 2022 08:25
@k8s-ci-robot k8s-ci-robot added the sig/release Categorizes an issue or PR as relevant to SIG Release. label Jun 13, 2022
@k8s-ci-robot k8s-ci-robot requested a review from Verolop June 13, 2022 08:25
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 13, 2022
@saschagrunert
Copy link
Member Author

cc @wilsonehusin

@saschagrunert
Copy link
Member Author

@cpanato PTAL, I think we also require a new release after this PR got merged.

@cpanato
Copy link
Member

cpanato commented Jun 13, 2022

will we get this back when the issue in upstream is fixed?

@saschagrunert
Copy link
Member Author

will we get this back when the issue in upstream is fixed?

Yes, but since the PR is open for a long time and the project overall seems to have got no modifications in the last couple of years I doubt that it will get merged soon.

Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [cpanato,saschagrunert]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 13, 2022
@cpanato
Copy link
Member

cpanato commented Jun 13, 2022

@saschagrunert i will run the release, sounds good?

@saschagrunert
Copy link
Member Author

@saschagrunert i will run the release, sounds good?

Awesome, thank you!

@k8s-ci-robot k8s-ci-robot merged commit 3e68251 into kubernetes-sigs:main Jun 13, 2022
@saschagrunert saschagrunert deleted the httpcache branch June 13, 2022 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/release Categorizes an issue or PR as relevant to SIG Release. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants