Added more clients #37
Conversation
| ConductorClientResponse<List<WebhookConfig>> response = getAllWebhooks(); | ||
| return response != null && response.getData() != null ? response.getData() : List.of(); | ||
| } catch (Exception e) { | ||
| return List.of(); |
There was a problem hiding this comment.
Is it intentional? For me as a developer it would be very confusing.
| public ConductorClientResponse<WebhookConfig> updateWebhook(String id, WebhookConfig webhookConfig) { | ||
| ConductorClientRequest request = ConductorClientRequest.builder() | ||
| .method(Method.PUT) | ||
| .path("/metadata/webhook/" + id) |
There was a problem hiding this comment.
nit: mostly in the code pathParam is used
| return client.execute(request, new TypeReference<>() { | ||
| }); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
nit: new line. Doesn't spotless should check it? I'll take a look later
|
|
||
| private String key; | ||
| private String value; | ||
| public String key; |
There was a problem hiding this comment.
Why do you need this? getter/setter is usually preferred in Java
| private String createdBy; | ||
| private List<Tag> tags; | ||
|
|
||
| private List<WebhookExecutionHistory> webhookExecutionHistory;//TODO Remove this |
There was a problem hiding this comment.
What is this TODO? Should it be merged? If yes, then I'd add to the comment info about when and why it should be removed/
| // Test error scenarios | ||
|
|
||
| // Test getting non-existent webhook | ||
| try { |
There was a problem hiding this comment.
Seems like tests are mostly generated by LLM. I don't think they are maintainable. Are we ok with having lots of working but not maintainable tests.
IvanKulik-sm
left a comment
IvanKulik-sm
left a comment
There was a problem hiding this comment.
- catch (){return } - I'm against it, let's discuss why we are doing it.
- E2E tests - are we ok with this LLM generated tests?
Pull Request type
NOTE: Please remember to run
./gradlew spotlessApplyto fix any format violations.Changes in this PR
Added SharedResourceClient and WebhookClient
Describe the new behavior from this PR, and why it's needed
Issue #
Alternatives considered
Describe alternative implementation you have considered