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

platform-checks: Perform validate() twice in the CI #20901

Merged

Conversation

philip-stoev
Copy link
Contributor

Make sure at least one step in the per-push CI performs validate() twice. This will weed out any validate()s that are not idempotent.

Previously, multiple validate() calls were only present in Nightly.

Motivation

Nightly CI was failing if non-idempotent validate()s are introduced.

Comment on lines +165 to +166
# Validate again so that introducing non-idempotent validate()s
# will cause the CI to fail.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice 👍

Comment on lines 542 to 552
+ self._drop_objects("owner_role_01", 1, expensive=True)
+ self._drop_objects("other_owner", 2, expensive=True)
+ self._drop_objects("owner_role_01", 3)
+ self._drop_objects("other_owner", 4)
+ self._drop_objects("owner_role_01", 5)
+ self._drop_objects("other_owner", 6)
+ self._drop_objects("owner_role_02", 7)
+ self._drop_objects("other_owner", 8)
+ self._drop_objects("owner_role_01", 9)
+ self._drop_objects("owner_role_02", 10)
+ self._drop_objects("owner_role_03", 11)
Copy link
Contributor

Choose a reason for hiding this comment

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

This was removed because it is in the validate part, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it was doing DROP in the validate() section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or maybe I misunderstood the entire test. It still fails anyway, so there is more to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nevermind, figured it out. all green now.

@philip-stoev philip-stoev force-pushed the platform-checks-validate-twice branch from 0873daa to 4d3b059 Compare August 2, 2023 07:46
Make sure at least one step in the per-push CI performs validate()
twice. This will weed out any validate()s that are not
idempotent.

Previously, multiple validate() calls were only present in Nightly.
@philip-stoev philip-stoev force-pushed the platform-checks-validate-twice branch from 4d3b059 to ba709aa Compare August 3, 2023 06:17
The validate() method needs to be idempotent
@philip-stoev philip-stoev force-pushed the platform-checks-validate-twice branch from ba709aa to 966559d Compare August 3, 2023 06:47
@philip-stoev philip-stoev merged commit 9911728 into MaterializeInc:main Aug 3, 2023
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