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

refactor!(server/pipeline): raise UndefinedVariableError when variables are missing during rendering [TCTC-10013] #2361

Merged

Conversation

Fanaen
Copy link
Contributor

@Fanaen Fanaen commented Mar 3, 2025

When there is a pipeline with a variable and a value is not provided, we get this error:

E       pydantic_core._pydantic_core.ValidationError: 3 validation errors for FilterStep
E       condition.ConditionComboAnd.and
E         Field required [type=missing, input_value={'column': 'price', 'operator': 'lt'}, input_type=dict]
E           For further information visit https://errors.pydantic.dev/2.10/v/missing
E       condition.ConditionComboOr.or
E         Field required [type=missing, input_value={'column': 'price', 'operator': 'lt'}, input_type=dict]
E           For further information visit https://errors.pydantic.dev/2.10/v/missing
E       condition.tagged-union[ComparisonCondition,ComparisonCondition,ComparisonCondition,ComparisonCondition,ComparisonCondition,ComparisonCondition,InclusionCondition,InclusionCondition,NullCondition,NullCondition,MatchCondition,MatchCondition,DateBoundCondition,DateBoundCondition].lt.value
E         Field required [type=missing, input_value={'column': 'price', 'operator': 'lt'}, input_type=dict]
E           For further information visit https://errors.pydantic.dev/2.10/v/missing

This PR uses an already there param in toucan-connectors::nosql_apply_parameters_to_query to get UndefinedVariableError instead.

This PR adds nosql_apply_parameters_to_query in weaverbird.utils.toucan_connectors to use this param everywhere, and a test to match.

@Fanaen Fanaen added bug Something isn't working fix labels Mar 3, 2025
@Fanaen Fanaen self-assigned this Mar 3, 2025
Copy link
Contributor

@lukapeschke lukapeschke left a comment

Choose a reason for hiding this comment

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

Thanks! Could you please also add an entry to the changelog ?

Also, the PR title isn't very explicit to me, how about something like "refactor!(server/pipeline): raise UndefinedVariableError when variables are missing during rendering"

@Fanaen Fanaen changed the title fix(toucan_connectors): get UndefinedVariableErrors instead of validation error [TCTC-10013] refactor!(server/pipeline): raise UndefinedVariableError when variables are missing during rendering [TCTC-10013] Mar 3, 2025
@Fanaen Fanaen requested a review from a team as a code owner March 3, 2025 14:42
@Fanaen Fanaen requested review from lukapeschke and julien-pinchelimouroux and removed request for a team March 3, 2025 14:42
@Fanaen
Copy link
Contributor Author

Fanaen commented Mar 3, 2025

Thanks for the review !
Would these changelogs be adequate: 01bc2ce?

Copy link
Contributor

@lukapeschke lukapeschke left a comment

Choose a reason for hiding this comment

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

thx!

Co-authored-by: Luka Peschke <[email protected]>
@Fanaen Fanaen merged commit 22fea2e into master Mar 3, 2025
4 checks passed
@Fanaen Fanaen deleted the TCTC-10013-v-3-difference-between-prod-and-staging-behaviour branch March 3, 2025 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants