-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
#356 added endpoint to fetch all cluster connectors #361
#356 added endpoint to fetch all cluster connectors #361
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
status: | ||
type: string | ||
enum: | ||
- RUNNING |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You already have this status enum, please just share it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks
type: | ||
type: string | ||
enum: | ||
- source |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use camel case for enums
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks
@@ -43,6 +53,58 @@ | |||
); | |||
} | |||
|
|||
public Flux<FullConnectorInfo> getAllConnectors(String clusterName) { | |||
return getConnects(clusterName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method looks overcomplicated, could we split it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks
FullConnectorInfo.StatusEnum.valueOf(triple.getLeft().getStatus().getState().name()) | ||
) | ||
.tasksCount(triple.getRight().size()) | ||
.hasFailedTasks(triple.getRight().stream() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we pass number of failed tasks instead? I think it will be more useful
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks
.collectList() | ||
.map(tasks -> Triple.of(pair.getLeft(), pair.getRight(), tasks)) | ||
) | ||
.map(triple -> new FullConnectorInfo() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please move it into mapper?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks
- refactoring schema - added connector class property
Kudos, SonarCloud Quality Gate passed! |
Kudos, SonarCloud Quality Gate passed!
|
…us#361) * added endpoint to fetch all cluster connectors * - refactoring code - refactoring schema - added connector class property
Fixes #356