Skip to content

Conversation

@tlento
Copy link
Collaborator

@tlento tlento commented Jun 30, 2023

The validation hook associated with SemanticModelDefaults would
register an error state if the optional agg_time_dimension property
was not set.

There was also a stray classmethod that really ought to be a staticmethod.
Forgive the "while I'm here" fixing but it doesn't seem worth a separate
commit.

@cla-bot cla-bot bot added the cla:yes label Jun 30, 2023
@github-actions
Copy link

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

The validation hook associated with SemanticModelDefaults would
register an error state if the optional agg_time_dimension property
was not set.

There was also a stray classmethod that really ought to be a staticmethod.
Forgive the "while I'm here" fixing but it doesn't seem worth a separate
commit.
@tlento tlento force-pushed the cleanup-validator-things branch from cb4ba18 to 78276af Compare June 30, 2023 17:00
@tlento tlento requested review from QMalcolm and WilliamDee June 30, 2023 17:00
Copy link
Collaborator

@QMalcolm QMalcolm left a comment

Choose a reason for hiding this comment

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

Ahh I see what was happening. The validation assumed that if defaults was specified, an agg time dim was required, when it's not. The change makes complete sense. Thank you for catching this and fixing it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants