Skip to content

feat: Add support to use ETag for conditional requests against the Github API#6505

Merged
zroubalik merged 9 commits intokedacore:mainfrom
andrewhibbert:feat_etags
Feb 18, 2025
Merged

feat: Add support to use ETag for conditional requests against the Github API#6505
zroubalik merged 9 commits intokedacore:mainfrom
andrewhibbert:feat_etags

Conversation

@andrewhibbert
Copy link
Contributor

@andrewhibbert andrewhibbert commented Jan 31, 2025 •
edited
Loading

Adds functionality to use conditional requests with the Github API by adding etag to request headers. If the response has not changed a 304 response is returned, which does not count against the primary rate limit.

Checklist

  • [N/A] When introducing a new scaler, I agree with the scaling governance policy
  • I have verified that my change is according to the deprecations & breaking changes policy
  • Tests have been added
  • Changelog has been updated and is aligned with our changelog requirements
  • [N/A] A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
  • A PR is opened to update the documentation on (repo) (if applicable)
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #6503

Relates to kedacore/keda-docs#1532

@andrewhibbert
feat: Add support to use ETag for conditional requests against the Gi…
feaaff1 
…thub API

Signed-off-by: andrewhibbert <a.hibbert@elsevier.com>
@andrewhibbert
feat: Add support to use ETag for conditional requests against the Gi…
415ec27 
…thub API

Signed-off-by: andrewhibbert <a.hibbert@elsevier.com>
@andrewhibbert andrewhibbert requested a review from a team as a code owner January 31, 2025 20:32
@andrewhibbert andrewhibbert changed the title Feat etags feat: Add support to use ETag for conditional requests against the Github API Jan 31, 2025
@andrewhibbert
feat: Add support to use ETag for conditional requests against the Gi…
f4f15fd 
…thub API

Signed-off-by: andrewhibbert <a.hibbert@elsevier.com>
@andrewhibbert
feat: Add support to use ETag for conditional requests against the Gi…
7dda86a 
…thub API

Signed-off-by: andrewhibbert <a.hibbert@elsevier.com>
@andrewhibbert
feat: Add support to use ETag for conditional requests against the Gi…
6a64ba3 
…thub API

Signed-off-by: andrewhibbert <a.hibbert@elsevier.com>
semgrep-app[bot]
semgrep-app bot reviewed Feb 3, 2025
View reviewed changes
JorTurFer
JorTurFer reviewed Feb 3, 2025
View reviewed changes
Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

This feature looks super nice! Thanks for the improvement 😄
Could you enable the etag feature in one of the github e2e test cases to keep it covered with e2e tests? I don't think that we need to add another fully e2e test case but just update one of the already existing to include this feature.
We'd need that you open a PR to docs adding this new parameter to the scaler docs 🙏

@andrewhibbert andrewhibbert mentioned this pull request Feb 4, 2025
Merged
@andrewhibbert
feat: Add support to use ETag for conditional requests against the Gi…
5f42fa6 
…thub API

Signed-off-by: andrewhibbert <a.hibbert@elsevier.com>
@andrewhibbert
Copy link
Contributor Author

andrewhibbert commented Feb 4, 2025

This feature looks super nice! Thanks for the improvement 😄 Could you enable the etag feature in one of the github e2e test cases to keep it covered with e2e tests? I don't think that we need to add another fully e2e test case but just update one of the already existing to include this feature. We'd need that you open a PR to docs adding this new parameter to the scaler docs 🙏

Thanks. I have opened a PR in the docs repo and changed one of the e2e testcase to have enabledEtags set to true

@JorTurFer
Copy link
Member

JorTurFer commented Feb 4, 2025 •
edited by github-actions bot
Loading

/run-e2e github
Update: You can check the progress here

@JorTurFer JorTurFer mentioned this pull request Feb 4, 2025
Closed
40 tasks
zroubalik
zroubalik reviewed Feb 6, 2025
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

Great stuff, thanks! Let's just fix the semgrep and we shoud be good to merge

@andrewhibbert
feat: Add support to use ETag for conditional requests against the Gi…
ad0c43c 
…thub API

Signed-off-by: andrewhibbert <a.hibbert@elsevier.com>
@andrewhibbert
feat: Add support to use ETag for conditional requests against the Gi…
361fb80 
…thub API

Signed-off-by: andrewhibbert <a.hibbert@elsevier.com>
@andrewhibbert
feat: Add support to use ETag for conditional requests against the Gi…
c544500 
…thub API

Signed-off-by: andrewhibbert <a.hibbert@elsevier.com>
@andrewhibbert
Copy link
Contributor Author

andrewhibbert commented Feb 13, 2025 •
edited
Loading

I have noticed there were some merge conflicts after #6519 was merged so I have made some changes to fit in with that change

@JorTurFer
Copy link
Member

JorTurFer commented Feb 14, 2025 •
edited by github-actions bot
Loading

/run-e2e github
Update: You can check the progress here

@zroubalik zroubalik merged commit 1666597 into kedacore:main Feb 18, 2025
19 checks passed
mittalvaibhav1 pushed a commit to mittalvaibhav1/keda that referenced this pull request Apr 26, 2025
@andrewhibbert @mittalvaibhav1
feat: Add support to use ETag for conditional requests against the Gi…
f217cad 
…thub API (kedacore#6505)

Signed-off-by: andrewhibbert <a.hibbert@elsevier.com>
Signed-off-by: mittalvaibhav1 <mittalvaibhavandroid@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zroubalik zroubalik zroubalik approved these changes

@JorTurFer JorTurFer Awaiting requested review from JorTurFer

+1 more reviewer

@semgrep-app semgrep-app[bot] semgrep-app[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Use conditional requests functionaliy with Github Scaler

3 participants

@andrewhibbert @JorTurFer @zroubalik