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

feat(config) make timeout configurable for the kong client #1401

Merged
merged 4 commits into from
Jun 11, 2021
Merged

feat(config) make timeout configurable for the kong client #1401

merged 4 commits into from
Jun 11, 2021

Conversation

tharun208
Copy link
Contributor

@tharun208 tharun208 commented Jun 7, 2021

Signed-off-by: Tharun [email protected]

What this PR does / why we need it: make the timeout seconds configurable in cached_proxy_resolver

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #1291

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@tharun208 tharun208 requested a review from a team as a code owner June 7, 2021 20:50
@CLAassistant
Copy link

CLAassistant commented Jun 7, 2021

CLA assistant check
All committers have signed the CLA.

railgun/pkg/config/config.go Outdated Show resolved Hide resolved
railgun/pkg/config/config.go Show resolved Hide resolved
Copy link
Contributor

@shaneutt shaneutt left a comment

Choose a reason for hiding this comment

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

Just one small thing with the unit test for your considerations but otherwise looking good 👍

@tharun208 tharun208 changed the base branch from main to next June 11, 2021 16:19
@tharun208 tharun208 requested a review from shaneutt June 11, 2021 16:19
Copy link
Contributor

@shaneutt shaneutt left a comment

Choose a reason for hiding this comment

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

Once CI is happy, I am happy 👍 🖖

@codecov
Copy link

codecov bot commented Jun 11, 2021

Codecov Report

Merging #1401 (126d609) into next (54c8000) will increase coverage by 0.12%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #1401      +/-   ##
==========================================
+ Coverage   55.08%   55.20%   +0.12%     
==========================================
  Files          42       42              
  Lines        3660     3661       +1     
==========================================
+ Hits         2016     2021       +5     
+ Misses       1493     1491       -2     
+ Partials      151      149       -2     
Impacted Files Coverage Δ
...n/internal/proxy/clientgo_cached_proxy_resolver.go 67.96% <100.00%> (+4.23%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54c8000...126d609. Read the comment docs.

@shaneutt
Copy link
Contributor

The licensing CI failure is a misnomer, this change doesn't break licensing rules.

@tharun208
Copy link
Contributor Author

this proxy_cache_resolver_test is flaky and this happens in my local too. can we address this in this same PR or a seperate one ?

@shaneutt shaneutt merged commit 93833d7 into Kong:next Jun 11, 2021
@tharun208 tharun208 deleted the feat/config_for_timeout branch June 11, 2021 19:17
@shaneutt shaneutt mentioned this pull request Jul 7, 2021
1 task
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.

Make the /config timeout configurable
3 participants