8000 fix: fetch interval should match default by ivarconr · Pull Request #281 · Unleash/unleash-client-java · GitHub
[go: up one dir, main page]

Skip to content

fix: fetch interval should match default #281

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

Merged
merged 4 commits into from
Feb 27, 2025
Merged

Conversation

ivarconr
Copy link
Member
@ivarconr ivarconr commented Feb 11, 2025

About the changes

  • fetch flag calls include unleash-interval header
  • send metrics calls include unleash-interval header
  • unleash-interval header is included even when it's set to 0 (but when it's is 0 then no API calls should be made)
  • registration calls has connectionId added to payload (previously it had unleash-connection-id header only)
  • send metrics has connectionId added to payload (previously it had unleash-connection-id header only)

@kwasniew kwasniew self-requested a review February 11, 2025 10:57
Copy link
Contributor
@kwasniew kwasniew left a comment

Choose a reason for hiding this comment

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

Probably we need to update some tests

@ivarconr ivarconr requested a review from chriswk February 11, 2025 12:20
@sjaanus sjaanus requested a review from kwasniew February 27, 2025 11:12
@coveralls
Copy link
Collaborator
coveralls commented Feb 27, 2025

Coverage Status

coverage: 79.623% (-0.2%) from 79.78%
when pulling 7ae7325 on fix/default-fetch-interval
into 136b205 on main.

@@ -236,6 +238,10 @@ public long getFetchTogglesInterval() {
return fetchTogglesInterval;
}

public String getFetchTogglesIntervalMillis() {
return String.valueOf(fetchTogglesInterval * 1000);
Copy link
Contributor

Choose a reason for hiding this comment

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

is it because config is in seconds and we report ms in the header?

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct

@sjaanus sjaanus merged commit 1a4fd77 into main Feb 27, 2025
10 checks passed
@sjaanus sjaanus deleted the fix/default-fetch-interval branch February 27, 2025 11:25
@github-project-automation github-project-automation bot moved this from Approved PRs to Done in Issues and PRs Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants
0