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(#1939): implement UI button to retrigger service detection #3938

Merged

Conversation

SteKoe
Copy link
Contributor

@SteKoe SteKoe commented Dec 23, 2024

Implements an POST endpoint /applications that triggers refreshing instances from discovery client. A UI button calls the new endpoint using a "confirm button".

screenshot

@SteKoe SteKoe requested a review from a team as a code owner December 23, 2024 13:54
@SteKoe SteKoe changed the title feat(1939): implement UI button to retrigger service detection feat(#1939): implement UI button to retrigger service detection Dec 23, 2024
Copy link
Contributor

@ulischulte ulischulte left a comment

Choose a reason for hiding this comment

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

LGTM

@SteKoe SteKoe merged commit 2e09fd9 into master Dec 23, 2024
2 checks passed
@SteKoe SteKoe deleted the feat/1939-implement-ui-button-to-retrigger-service-detection branch December 23, 2024 15:15
}

@GetMapping(path = "/applications", produces = MediaType.APPLICATION_JSON_VALUE)
public Flux<Application> applications() {
return registry.getApplications();
}

@PostMapping(path = "/applications", produces = MediaType.APPLICATION_JSON_VALUE)
public void refreshApplications() {
Copy link
Member

Choose a reason for hiding this comment

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

I know this has already been merged, but I`m no sure if POST to /applications is intuitive for triggering discovery.

I would expect to send some data and add a new application on that method+path (like registering). That would also match the DELETE below where we unregister an application.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants