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

CI checks to check for missing downgrade updates #6661

Merged
merged 1 commit into from
Feb 15, 2023
Merged

Conversation

hanefi
Copy link
Member

@hanefi hanefi commented Jan 27, 2023

A branch that touches a set of upgrade scripts is also expected to touch corresponding downgrade scripts as well. To ensure that I introduce a new CI script. If this script fails, read the output and make sure you update the downgrade scripts in the printed list.

@hanefi hanefi changed the title Install downgrade scripts by default CI checks to check for missing downgrade updates Jan 27, 2023
@hanefi
Copy link
Member Author

hanefi commented Jan 27, 2023

There is a small problem with grep on CI machines. It is busybox grep that does not accept long parameter names. Hence I needed to install GNU grep in the CI images. See citusdata/the-process#105

ci/README.md Outdated Show resolved Hide resolved
hanefi added a commit to citusdata/the-process that referenced this pull request Feb 7, 2023
When we add some scripts to run in CI, it is a nice idea to use long
argument names. However, the busybox grep binary in our images do not
support long argument names, and hence we install GNU grep.

Used in: citusdata/citus#6661
.circleci/config.yml Outdated Show resolved Hide resolved
@hanefi hanefi changed the base branch from install-downgrades-by-default to main February 7, 2023 20:00
@hanefi
Copy link
Member Author

hanefi commented Feb 7, 2023

I used to base these changes here on top of #6637. I now rebased on top of main and changed the target branch of the PR accordingly.

Copy link
Contributor

@JelteF JelteF left a comment

Choose a reason for hiding this comment

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

LGTM, but it's not doing the check for columnar upgrade/downgrades. This is good to merge as is though, we can add columnar support in another PR.

Comment on lines +10 to +11
upgrade_files=$(git diff --name-only origin/main | { grep "src/backend/distributed/sql/citus--.*sql" || exit 0 ; })
downgrade_files=$(git diff --name-only origin/main | { grep "src/backend/distributed/sql/downgrades/citus--.*sql" || exit 0 ; })
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only doing this check for citus upgrade files, not for columnar ones.

@hanefi hanefi merged commit 902d426 into main Feb 15, 2023
@hanefi hanefi deleted the downgrade-checks branch February 15, 2023 15:20
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