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

feat: annotations property for models #422

Merged
merged 3 commits into from
Dec 2, 2023

Conversation

leecalcote
Copy link
Member

@leecalcote leecalcote commented Dec 2, 2023

Signed-off-by: Lee Calcote [email protected]

Description

This PR adds a new Annotations property to Model Filters

Notes for Reviewers

Signed commits

  • Yes, I signed my commits.

Signed-off-by: Lee Calcote <[email protected]>
Comment on lines +346 to +350
if mf.Annotations == "true" {
finder = finder.Where("model_dbs.metadata->>'isAnnotation' = true")
} else if mf.Annotations == "false" {
finder = finder.Where("model_dbs.metadata->>'isAnnotation' = false")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if that is intentional, if we are making it to accept string we may pass "inlude", "exclude" or one more can't remember, instead of boolean as string -> "true"

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, good point, @Yashsharma1911. I agree that what you've suggested is a desired improvement.

@MUzairS15
Copy link

updated

@leecalcote leecalcote merged commit 5c52cbb into master Dec 2, 2023
4 checks passed
@leecalcote leecalcote deleted the leecalcote/models/annotations branch December 2, 2023 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants