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

Add SetTag endpoint #10

Merged
merged 27 commits into from
Oct 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
41c7754
Add SetTag endpoint
jescalada Oct 4, 2024
97927f6
Fix line endings to LF
jescalada Oct 4, 2024
2cc3ee1
Update mock store
jescalada Oct 8, 2024
59f30ac
Clean up unused code
jescalada Oct 8, 2024
a3235d3
Refactor tests.go
jescalada Oct 8, 2024
b810993
Remove unused validation code
jescalada Oct 8, 2024
084c471
Add SetTag struct validation
jescalada Oct 9, 2024
9039d3f
Replace runId requirement with manual check
jescalada Oct 9, 2024
fd44cbc
Clean up SetTag store
jescalada Oct 9, 2024
4344210
Minor adjustments
jescalada Oct 9, 2024
02ed6d9
Update validations
jescalada Oct 9, 2024
bc25c96
Add preliminary SetTag missing logic
jescalada Oct 11, 2024
7086ea1
Add missing generated file
jescalada Oct 11, 2024
5bc3cd0
Merge remote-tracking branch 'origin/main' into implement-set-tag
jescalada Oct 11, 2024
8eeaacb
Fix protoc version issue
jescalada Oct 11, 2024
91660ed
Fix test_update_run_name test
jescalada Oct 14, 2024
f91e43b
Add pythonSpecific to docs, normalize MLflow spelling
jescalada Oct 14, 2024
a3bd537
Override test_set_tag execution
jescalada Oct 16, 2024
b39b3f0
Merge remote-tracking branch 'origin/main' into implement-set-tag
jescalada Oct 16, 2024
19404be
Extract helper functions from SetTag
jescalada Oct 16, 2024
87c2efe
Revert unnecessary changes
jescalada Oct 16, 2024
a7b7390
Fix postCreate.sh
jescalada Oct 16, 2024
b27e4f6
Fix linter error
jescalada Oct 16, 2024
fe585cc
Fix format issue
jescalada Oct 16, 2024
2f753ad
Simplify handleRunNameUpdate
jescalada Oct 19, 2024
d6081fd
Merge branch 'main' into implement-set-tag
jescalada Oct 19, 2024
069212c
Refactor PythonSpecific
nojaf Oct 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions magefiles/generate/validations.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ var validations = map[string]string{
"LogMetric_Key": "required",
"LogMetric_Value": "required",
"LogMetric_Timestamp": "required",
"SetTag_RunId": "required",
"SetTag_Key": "required",
"DeleteTag_RunId": "required",
"DeleteTag_Key": "required",
"SetTag_Key": "required",
"DeleteTag_RunId": "required",
"DeleteTag_Key": "required",
}
2 changes: 1 addition & 1 deletion pkg/protos/service.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion pkg/tracking/service/tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@ import (
)

func (ts TrackingService) SetTag(ctx context.Context, input *protos.SetTag) (*protos.SetTag_Response, *contract.Error) {
if err := ts.Store.SetTag(ctx, input.GetRunId(), input.GetKey(), input.GetValue()); err != nil {
runID := input.GetRunId()

if runID == "" {
runID = input.GetRunUuid()
}

if err := ts.Store.SetTag(ctx, runID, input.GetKey(), input.GetValue()); err != nil {
return nil, err
}

Expand Down
9 changes: 0 additions & 9 deletions pkg/tracking/store/sql/tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"
"strconv"

"gorm.io/gorm"
"gorm.io/gorm/clause"
Expand Down Expand Up @@ -92,14 +91,6 @@ func (s TrackingSQLStore) SetTag(
)
}

// If the runID can be parsed as a number, it should throw an error
if _, err := strconv.ParseFloat(runID, 64); err == nil {
return contract.NewError(
protos.ErrorCode_INVALID_PARAMETER_VALUE,
fmt.Sprintf("Invalid value %s for parameter 'run_id' supplied", runID),
)
}

err := s.db.WithContext(ctx).Transaction(func(transaction *gorm.DB) error {
contractError := checkRunIsActive(transaction, runID)
if contractError != nil {
Expand Down
10 changes: 10 additions & 0 deletions pkg/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,15 @@ func validateLogBatchLimits(structLevel validator.StructLevel) {
}
}

// SetTag must have either a run_id or a run_uuid present.
func validateSetTagRunIDExists(structLevel validator.StructLevel) {
jescalada marked this conversation as resolved.
Show resolved Hide resolved
tag, isTag := structLevel.Current().Interface().(*protos.SetTag)

if isTag && tag.GetRunId() == "" && tag.GetRunUuid() == "" {
structLevel.ReportError(&tag, "run_id", "", "", "")
}
}

func truncateFn(fieldLevel validator.FieldLevel) bool {
param := fieldLevel.Param() // Get the parameter from the tag

Expand Down Expand Up @@ -188,6 +197,7 @@ func NewValidator() (*validator.Validate, error) {
}

validate.RegisterStructValidation(validateLogBatchLimits, &protos.LogBatch{})
validate.RegisterStructValidation(validateSetTagRunIDExists, &protos.SetTag{})

return validate, nil
}
Expand Down