-
Notifications
You must be signed in to change notification settings - Fork 313
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
[OPIK-757] Add support for comments part2 #1121
[OPIK-757] Add support for comments part2 #1121
Conversation
apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/TracesResource.java
Show resolved
Hide resolved
.bind("entity_type", entityType.getType()); | ||
|
||
return makeMonoContextAware(bindWorkspaceIdToMono(statement)) | ||
.then(); |
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.
It is better to return the affected rows so the client can decide whether to do something about it or not
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.
I will update in the next PR, good point. Yet in most DAO's in project now we return Void.
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.
It makes the function more reusable.
import static com.comet.opik.utils.ValidationUtils.CLICKHOUSE_FIXED_STRING_UUID_FIELD_NULL_VALUE; | ||
|
||
@UtilityClass | ||
public class CommentResultMapper { |
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.
public class CommentResultMapper { | |
class CommentResultMapper { |
Details
Add support for comments
Testing
Integration tests