-
Notifications
You must be signed in to change notification settings - Fork 210
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
[Online Scoring] Automation Rules endpoints #945
base: main
Are you sure you want to change the base?
Changes from all commits
1315442
59cb8c2
bdf11fe
6d731f6
eccaa4b
a79b400
a20a452
a544940
84f5197
f4310cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
package com.comet.opik.api; | ||
|
||
import com.fasterxml.jackson.annotation.JsonValue; | ||
import lombok.AccessLevel; | ||
import lombok.Getter; | ||
import lombok.RequiredArgsConstructor; | ||
|
||
import java.time.Instant; | ||
import java.util.Arrays; | ||
import java.util.UUID; | ||
|
||
public sealed interface AutomationRule<T> permits AutomationRuleEvaluator { | ||
|
||
UUID id(); | ||
UUID projectId(); | ||
|
||
AutomationRuleAction action(); | ||
float samplingRate(); | ||
|
||
Instant createdAt(); | ||
String createdBy(); | ||
Instant lastUpdatedAt(); | ||
String lastUpdatedBy(); | ||
|
||
@Getter | ||
@RequiredArgsConstructor(access = AccessLevel.PRIVATE) | ||
enum AutomationRuleAction { | ||
|
||
EVALUATOR("evaluator"); | ||
|
||
@JsonValue | ||
private final String action; | ||
|
||
public static AutomationRule.AutomationRuleAction fromString(String action) { | ||
return Arrays.stream(values()).filter(v -> v.action.equals(action)).findFirst() | ||
.orElseThrow(() -> new IllegalArgumentException("Unknown feedback type: " + action)); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
package com.comet.opik.api; | ||
|
||
import com.fasterxml.jackson.annotation.JsonIgnoreProperties; | ||
import com.fasterxml.jackson.annotation.JsonView; | ||
import com.fasterxml.jackson.databind.PropertyNamingStrategies; | ||
import com.fasterxml.jackson.databind.annotation.JsonNaming; | ||
import io.swagger.v3.oas.annotations.media.Schema; | ||
import jakarta.validation.constraints.NotNull; | ||
import jakarta.validation.constraints.Pattern; | ||
import lombok.Builder; | ||
|
||
import java.time.Instant; | ||
import java.util.List; | ||
import java.util.UUID; | ||
|
||
import static com.comet.opik.utils.ValidationUtils.NULL_OR_NOT_BLANK; | ||
|
||
@Builder(toBuilder = true) | ||
@JsonIgnoreProperties(ignoreUnknown = true) | ||
@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) | ||
public record AutomationRuleEvaluator ( | ||
// Fields and methods | ||
@JsonView({View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) UUID id, | ||
@JsonView({View.Public.class, View.Write.class}) @NotNull UUID projectId, | ||
@JsonView({View.Public.class, View.Write.class}) @NotNull AutomationRuleEvaluatorType type, | ||
|
||
@JsonView({View.Public.class, View.Write.class}) @Pattern(regexp = NULL_OR_NOT_BLANK, message = "must not be blank") String code, | ||
@JsonView({View.Public.class, View.Write.class}) float samplingRate, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is validated at the DAO level, please add validations on this API object as well. |
||
|
||
@JsonView({View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) Instant createdAt, | ||
@JsonView({View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) String createdBy, | ||
@JsonView({View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) Instant lastUpdatedAt, | ||
@JsonView({View.Public.class}) @Schema(accessMode = Schema.AccessMode.READ_ONLY) String lastUpdatedBy) implements AutomationRule { | ||
|
||
@Override | ||
public AutomationRuleAction action() { | ||
return AutomationRuleAction.EVALUATOR; | ||
} | ||
|
||
public static class View { | ||
public static class Write {} | ||
public static class Public {} | ||
} | ||
|
||
@Builder(toBuilder = true) | ||
public record AutomationRuleEvaluatorPage( | ||
@JsonView({View.Public.class}) int page, | ||
@JsonView({View.Public.class}) int size, | ||
@JsonView({View.Public.class}) long total, | ||
@JsonView({View.Public.class}) List<AutomationRuleEvaluator> content) | ||
implements Page<AutomationRuleEvaluator>{ | ||
|
||
public static AutomationRuleEvaluator.AutomationRuleEvaluatorPage empty(int page) { | ||
return new AutomationRuleEvaluator.AutomationRuleEvaluatorPage(page, 0, 0, List.of()); | ||
} | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: run spotless on your PRs, so code is automatically formatted. This is a previous miss from our side, we need to automate this. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package com.comet.opik.api; | ||
|
||
import com.fasterxml.jackson.annotation.JsonValue; | ||
import lombok.AccessLevel; | ||
import lombok.Getter; | ||
import lombok.RequiredArgsConstructor; | ||
|
||
@Getter | ||
@RequiredArgsConstructor(access = AccessLevel.PRIVATE) | ||
public enum AutomationRuleEvaluatorType { | ||
|
||
LLM_AS_JUDGE("llm-as-judge"), | ||
PYTHON("python"); | ||
|
||
@JsonValue | ||
private final String type; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package com.comet.opik.api; | ||
|
||
import com.fasterxml.jackson.annotation.JsonIgnoreProperties; | ||
import com.fasterxml.jackson.databind.PropertyNamingStrategies; | ||
import com.fasterxml.jackson.databind.annotation.JsonNaming; | ||
import jakarta.validation.constraints.NotBlank; | ||
import lombok.Builder; | ||
|
||
@Builder(toBuilder = true) | ||
@JsonIgnoreProperties(ignoreUnknown = true) | ||
@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) | ||
public record AutomationRuleEvaluatorUpdate( | ||
@NotBlank String code, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same, let's change |
||
float samplingRate) { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,168 @@ | ||
package com.comet.opik.api.resources.v1.priv; | ||
|
||
import com.codahale.metrics.annotation.Timed; | ||
import com.comet.opik.api.AutomationRuleEvaluator; | ||
import com.comet.opik.api.AutomationRuleEvaluatorUpdate; | ||
import com.comet.opik.api.Page; | ||
import com.comet.opik.domain.AutomationRuleEvaluatorService; | ||
import com.comet.opik.infrastructure.auth.RequestContext; | ||
import com.comet.opik.infrastructure.ratelimit.RateLimited; | ||
import com.fasterxml.jackson.annotation.JsonView; | ||
import io.swagger.v3.oas.annotations.Operation; | ||
import io.swagger.v3.oas.annotations.headers.Header; | ||
import io.swagger.v3.oas.annotations.media.Content; | ||
import io.swagger.v3.oas.annotations.media.Schema; | ||
import io.swagger.v3.oas.annotations.parameters.RequestBody; | ||
import io.swagger.v3.oas.annotations.responses.ApiResponse; | ||
import io.swagger.v3.oas.annotations.tags.Tag; | ||
import jakarta.inject.Inject; | ||
import jakarta.inject.Provider; | ||
import jakarta.validation.Valid; | ||
import jakarta.validation.constraints.Min; | ||
import jakarta.validation.constraints.NotNull; | ||
import jakarta.ws.rs.Consumes; | ||
import jakarta.ws.rs.DELETE; | ||
import jakarta.ws.rs.DefaultValue; | ||
import jakarta.ws.rs.GET; | ||
import jakarta.ws.rs.POST; | ||
import jakarta.ws.rs.PUT; | ||
import jakarta.ws.rs.Path; | ||
import jakarta.ws.rs.PathParam; | ||
import jakarta.ws.rs.Produces; | ||
import jakarta.ws.rs.QueryParam; | ||
import jakarta.ws.rs.core.Context; | ||
import jakarta.ws.rs.core.MediaType; | ||
import jakarta.ws.rs.core.Response; | ||
import jakarta.ws.rs.core.UriInfo; | ||
import lombok.NonNull; | ||
import lombok.RequiredArgsConstructor; | ||
import lombok.extern.slf4j.Slf4j; | ||
|
||
import java.net.URI; | ||
import java.util.UUID; | ||
|
||
@Path("/v1/private/automation/evaluators/project/{projectId}") | ||
ldaugusto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@Produces(MediaType.APPLICATION_JSON) | ||
@Consumes(MediaType.APPLICATION_JSON) | ||
@Timed | ||
@Slf4j | ||
@RequiredArgsConstructor(onConstructor_ = @Inject) | ||
@Tag(name = "Automation rule evaluators", description = "Automation rule evaluators resource") | ||
public class AutomationRuleEvaluatorsResource { | ||
|
||
private final @NonNull AutomationRuleEvaluatorService service; | ||
private final @NonNull Provider<RequestContext> requestContext; | ||
|
||
@GET | ||
@Operation(operationId = "findEvaluators", summary = "Find Evaluators", description = "Find Evaluators", responses = { | ||
@ApiResponse(responseCode = "200", description = "Evaluators resource", content = @Content(schema = @Schema(implementation = AutomationRuleEvaluator.AutomationRuleEvaluatorPage.class))) | ||
}) | ||
@JsonView(AutomationRuleEvaluator.View.Public.class) | ||
public Response find(@PathParam("projectId") UUID projectId, | ||
@QueryParam("page") @Min(1) @DefaultValue("1") int page, | ||
@QueryParam("size") @Min(1) @DefaultValue("10") int size) { | ||
|
||
String workspaceId = requestContext.get().getWorkspaceId(); | ||
log.info("Looking for automated evaluators for project id '{}' on workspaceId '{}' (page {})", projectId, | ||
workspaceId, page); | ||
Page<AutomationRuleEvaluator> definitionPage = service.find(page, size, projectId, workspaceId); | ||
log.info("Found {} automated evaluators for project id '{}' on workspaceId '{}' (page {}, total {})", | ||
definitionPage.size(), projectId, workspaceId, page, definitionPage.total()); | ||
|
||
return Response.ok() | ||
.entity(definitionPage) | ||
.build(); | ||
} | ||
|
||
@GET | ||
@Path("/evaluator/{evaluatorId}") | ||
ldaugusto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@Operation(operationId = "getEvaluatorById", summary = "Get automation rule evaluator by id", description = "Get automation rule by id", responses = { | ||
@ApiResponse(responseCode = "200", description = "Automation Rule resource", content = @Content(schema = @Schema(implementation = AutomationRuleEvaluator.class))) | ||
}) | ||
@JsonView(AutomationRuleEvaluator.View.Public.class) | ||
public Response getEvaluator(@PathParam("projectId") UUID projectId, @PathParam("evaluatorId") UUID evaluatorId) { | ||
String workspaceId = requestContext.get().getWorkspaceId(); | ||
|
||
log.info("Looking for automated evaluator: id '{}' on project_id '{}'", projectId, workspaceId); | ||
AutomationRuleEvaluator evaluator = service.findById(evaluatorId, projectId, workspaceId); | ||
log.info("Found automated evaluator: id '{}' on project_id '{}'", projectId, workspaceId); | ||
|
||
return Response.ok().entity(evaluator).build(); | ||
} | ||
|
||
@POST | ||
@Operation(operationId = "createAutomationRuleEvaluator", summary = "Create automation rule evaluator", description = "Create automation rule evaluator", responses = { | ||
@ApiResponse(responseCode = "201", description = "Created", headers = { | ||
@Header(name = "Location", required = true, example = "${basePath}/api/v1/private/automation/evaluators/project/{projectId}/evaluator/{evaluatorId}", schema = @Schema(implementation = String.class)) | ||
ldaugusto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}) | ||
}) | ||
@RateLimited | ||
public Response createEvaluator( | ||
@RequestBody(content = @Content(schema = @Schema(implementation = AutomationRuleEvaluator.class))) @JsonView(AutomationRuleEvaluator.View.Write.class) @NotNull @Valid AutomationRuleEvaluator evaluator, | ||
@Context UriInfo uriInfo) { | ||
|
||
String workspaceId = requestContext.get().getWorkspaceId(); | ||
String userName = requestContext.get().getUserName(); | ||
|
||
log.info("Creating {} evaluator for project_id '{}' on workspace_id '{}'", evaluator.type(), | ||
evaluator.projectId(), workspaceId); | ||
AutomationRuleEvaluator savedEvaluator = service.save(evaluator, workspaceId, userName); | ||
log.info("Created {} evaluator '{}' for project_id '{}' on workspace_id '{}'", evaluator.type(), | ||
savedEvaluator.id(), evaluator.projectId(), workspaceId); | ||
|
||
URI uri = uriInfo.getAbsolutePathBuilder() | ||
.path("/projectId/%s/evaluator/%s".formatted(savedEvaluator.projectId().toString(), | ||
savedEvaluator.id().toString())) | ||
.build(); | ||
return Response.created(uri).build(); | ||
} | ||
|
||
@PUT | ||
ldaugusto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@Path("/evaluator/{id}") | ||
@Operation(operationId = "updateAutomationRuleEvaluator", summary = "update Automation Rule Evaluator by id", description = "update Automation Rule Evaluator by id", responses = { | ||
@ApiResponse(responseCode = "204", description = "No content"), | ||
}) | ||
@RateLimited | ||
public Response updateEvaluator(@PathParam("id") UUID id, | ||
@PathParam("projectId") UUID projectId, | ||
@RequestBody(content = @Content(schema = @Schema(implementation = AutomationRuleEvaluatorUpdate.class))) @NotNull @Valid AutomationRuleEvaluatorUpdate evaluatorUpdate) { | ||
|
||
String workspaceId = requestContext.get().getWorkspaceId(); | ||
String userName = requestContext.get().getUserName(); | ||
|
||
log.info("Updating automation rule evaluator by id '{}' and project_id '{}' on workspace_id '{}'", id, | ||
projectId, workspaceId); | ||
service.update(id, projectId, workspaceId, userName, evaluatorUpdate); | ||
log.info("Updated automation rule evaluator by id '{}' and project_id '{}' on workspace_id '{}'", id, projectId, | ||
workspaceId); | ||
|
||
return Response.noContent().build(); | ||
} | ||
|
||
@DELETE | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're generally creating batch endpoints, so you can delete by sending multiple ids. |
||
@Path("/evaluator/{id}") | ||
@Operation(operationId = "deleteAutomationRuleEvaluatorById", summary = "Delete Automation Rule Evaluator by id", description = "Delete a single Automation Rule Evaluator by id", responses = { | ||
@ApiResponse(responseCode = "204", description = "No content"), | ||
}) | ||
public Response deleteEvaluator(@PathParam("id") UUID id, @PathParam("projectId") UUID projectId) { | ||
|
||
String workspaceId = requestContext.get().getWorkspaceId(); | ||
log.info("Deleting dataset by id '{}' on workspace_id '{}'", id, workspaceId); | ||
service.delete(id, projectId, workspaceId); | ||
log.info("Deleted dataset by id '{}' on workspace_id '{}'", id, workspaceId); | ||
return Response.noContent().build(); | ||
} | ||
|
||
@DELETE | ||
@Operation(operationId = "deleteAutomationRuleEvaluatorByProject", summary = "Delete all project Automation Rule Evaluators", description = "Delete all Automation Rule Evaluator in a project", responses = { | ||
@ApiResponse(responseCode = "204", description = "No content"), | ||
}) | ||
public Response deleteProjectEvaluators(@PathParam("projectId") UUID projectId) { | ||
|
||
String workspaceId = requestContext.get().getWorkspaceId(); | ||
log.info("Deleting evaluators from project_id '{}' on workspace_id '{}'", projectId, workspaceId); | ||
service.deleteByProject(projectId, workspaceId); | ||
log.info("Deleted evaluators from project_id '{}' on workspace_id '{}'", projectId, workspaceId); | ||
return Response.noContent().build(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
package com.comet.opik.domain; | ||
|
||
import com.comet.opik.api.AutomationRule; | ||
import com.comet.opik.api.AutomationRuleEvaluator; | ||
import com.comet.opik.infrastructure.db.UUIDArgumentFactory; | ||
import org.jdbi.v3.sqlobject.config.RegisterArgumentFactory; | ||
import org.jdbi.v3.sqlobject.config.RegisterConstructorMapper; | ||
import org.jdbi.v3.sqlobject.config.RegisterRowMapper; | ||
import org.jdbi.v3.sqlobject.customizer.Bind; | ||
import org.jdbi.v3.sqlobject.customizer.BindList; | ||
import org.jdbi.v3.sqlobject.customizer.BindMethods; | ||
import org.jdbi.v3.sqlobject.statement.SqlQuery; | ||
import org.jdbi.v3.sqlobject.statement.SqlUpdate; | ||
|
||
import java.util.Set; | ||
import java.util.UUID; | ||
|
||
@RegisterArgumentFactory(UUIDArgumentFactory.class) | ||
@RegisterRowMapper(AutomationRuleRowMapper.class) | ||
@RegisterConstructorMapper(AutomationRuleEvaluator.class) | ||
interface AutomationRuleDAO { | ||
|
||
@SqlUpdate("INSERT INTO automation_rules(id, project_id, workspace_id, `action`, sampling_rate, created_by, last_updated_by) "+ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: Is |
||
"VALUES (:rule.id, :rule.projectId, :workspaceId, :rule.action, :rule.samplingRate, :rule.createdBy, :rule.lastUpdatedBy)") | ||
void saveBaseRule(@BindMethods("rule") AutomationRule rule, @Bind("workspaceId") String workspaceId); | ||
|
||
@SqlUpdate(""" | ||
UPDATE automation_rules | ||
SET sampling_rate = :samplingRate, | ||
last_updated_by = :lastUpdatedBy | ||
WHERE id = :id AND project_id = :projectId AND workspace_id = :workspaceId | ||
""") | ||
int updateBaseRule(@Bind("id") UUID id, | ||
@Bind("projectId") UUID projectId, | ||
@Bind("workspaceId") String workspaceId, | ||
@Bind("samplingRate") float samplingRate, | ||
@Bind("lastUpdatedBy") String lastUpdatedBy); | ||
|
||
@SqlUpdate("DELETE FROM automation_rules WHERE id = :id AND project_id = :projectId AND workspace_id = :workspaceId") | ||
void delete(@Bind("id") UUID id, @Bind("projectId") UUID projectId, @Bind("workspaceId") String workspaceId); | ||
Comment on lines
+39
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary, as you have the version with multiple ids. |
||
|
||
@SqlUpdate("DELETE FROM automation_rules WHERE `action` = :action AND project_id = :projectId AND workspace_id = :workspaceId") | ||
void deleteByProject(@Bind("projectId") UUID projectId, | ||
@Bind("workspaceId") String workspaceId, | ||
@Bind("action") AutomationRule.AutomationRuleAction action); | ||
|
||
@SqlUpdate("DELETE FROM automation_rules WHERE id IN (<ids>) AND project_id = :projectId AND workspace_id = :workspaceId") | ||
void delete(@BindList("ids") Set<UUID> ids, @Bind("projectId") UUID projectId, @Bind("workspaceId") String workspaceId); | ||
Comment on lines
+42
to
+48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just use a template to have a single method, by making action and ids optional. |
||
|
||
@SqlQuery("SELECT COUNT(*) FROM automation_rules WHERE project_id = :projectId AND workspace_id = :workspaceId") | ||
long findCount(@Bind("projectId") UUID projectId, @Bind("workspaceId") String workspaceId); | ||
|
||
@SqlQuery("SELECT COUNT(*) FROM automation_rules WHERE project_id = :projectId AND workspace_id = :workspaceId AND `action` = :action") | ||
long findCountByActionType(@Bind("projectId") UUID projectId, @Bind("workspaceId") String workspaceId, @Bind("action") AutomationRule.AutomationRuleAction action); | ||
Comment on lines
+50
to
+54
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same, use a template to make action optional. We don't need two methods for the same thing. |
||
} |
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.
Let's make this interface and its subclasses self de/serialisable without requiring any external help. You just need to configure it with
JsonTypeInfo
andJsonSubTypes
annotations etc., based on theaction
property.