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

DHIS2-18568: allow Route response timeout to be customised #19872

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

cjmamo
Copy link
Contributor

@cjmamo cjmamo commented Feb 6, 2025

  • Replaced blocking route HTTP client with non-blocking client to remove connection pool
  • Allowing route response timeout to be customised

perf: use non-blocking WebClient instead of RestTemplate
tolerate HTTP errors in WebClient

test: add test coverage for response timeout and HTTP errors
@cjmamo cjmamo requested review from a team February 6, 2025 12:59
@cjmamo cjmamo self-assigned this Feb 6, 2025
@cjmamo cjmamo changed the title DHIS2-18568: allow response timeout to be customised DHIS2-18568: allow Route response timeout to be customised Feb 6, 2025
use int instead of Integer for responseTimeout to remove null checks

remove redundant annotations

rename references

use `Json.object` instead handcrafting JSON in RouteService
@amcgee amcgee added the deploy Deploy DHIS2 instance with IM. label Feb 10, 2025
Copy link

Instance deployed to https://dev.im.dhis2.org/pr-19872

Copy link
Member

@amcgee amcgee left a comment

Choose a reason for hiding this comment

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

LGTM! Would be great to get one more review from the backend team

@@ -48,6 +48,7 @@
@EqualsAndHashCode(callSuper = true)
@Accessors(chain = true)
public class Route extends BaseIdentifiableObject implements MetadataObject {
public static final String DEFAULT_RESPONSE_TIMEOUT_SECONDS = "5";
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: You can declare the constant as an int and use ""+DEFAULT_RESPONSE_TIMEOUT_SECONDS in the annotation to turn it into a string.

throwable -> {
JsonObject message =
Json.object(
jsonObjectBuilder ->
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd name this just obj or object because in the scope where the variable is valid it reads much better to pretend this is already an object. The "building" will happen implicitly at the end of the scope so it doesn't add anything to make the reader aware that the interface you are using is in fact a builder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy Deploy DHIS2 instance with IM.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants