Skip to content

api: Validate the profile type before ingestion#6061

Merged
brancz merged 5 commits intomainfrom
profile-type-validation
Nov 24, 2025
Merged

api: Validate the profile type before ingestion#6061
brancz merged 5 commits intomainfrom
profile-type-validation

Conversation

@manojVivek
Copy link
Contributor

No description provided.

@alwaysmeticulous
Copy link

alwaysmeticulous bot commented Nov 24, 2025

✅ Meticulous spotted 0 visual differences across 321 screens tested: view results.

Meticulous evaluated ~5 hours of user flows against your PR.

Expected differences? Click here. Last updated for commit ad225d8. This comment will update as new commits are pushed.

Copy link
Member

@brancz brancz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the “parse don’t validate” approach!

}

for rowIdx := 0; rowIdx < int(r.NumRows()); rowIdx++ {
profileType, err := getProfileTypeString(r, rowIdx)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function searches for the producer column on every iteration, let’s only find the column once and then on every iteration do this validation.


schema := r.Schema()

nameCol := r.Column(schema.FieldIndices(profile.ColumnName)[0])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is user-supplied data, so we shouldn't panic if there is no field index

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah makes sense. Fixing it.

@brancz brancz merged commit d1c706b into main Nov 24, 2025
37 of 39 checks passed
@brancz brancz deleted the profile-type-validation branch November 24, 2025 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants