From 28294087e0f5e51e850d284056904ace873a8fd4 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Fri, 5 Mar 2021 16:14:16 -0800 Subject: [PATCH 1/7] build: notify PR authors about potential db migration conflict --- .../workflows/check_db_migration_confict.yml | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 .github/workflows/check_db_migration_confict.yml diff --git a/.github/workflows/check_db_migration_confict.yml b/.github/workflows/check_db_migration_confict.yml new file mode 100644 index 000000000000..68d3a19c17b2 --- /dev/null +++ b/.github/workflows/check_db_migration_confict.yml @@ -0,0 +1,58 @@ +name: Check DB migration conflict +on: + push: + paths: + - "superset/migrations/**" + +jobs: + check_db_migration_conflict: + name: Check potential DB migration conflict + runs-on: ubuntu-20.04 + steps: + - name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )" + uses: actions/checkout@v2 + with: + persist-credentials: false + submodules: recursive + - name: Check and notify + uses: actions/github-script@v3 + with: + githubToken: ${{ secrets.GITHUB_TOKEN }} + script: | + // API reference: https://octokit.github.io/rest.js + + // Find all pull requests to current branch + const opts = github.pulls.list.endpoint.merge({ + owner: context.repo.owner, + repo: context.repo.repo, + base: contect.ref, + state: 'open', + sort: 'updated', + per_page: 100, + }); + const pulls = await github.paginate(opts); + + for (const pull in pulls) { + const listFilesOpts = await github.pulls.listFiles.endpoint.merge({ + owner: context.repo.owner, + repo: contet.repo.repo, + pull_number: pull.number, + }); + const files = await github.paginate(listFilesOpts); + if ( + files.some(x => x.contents_url.includes('/contents/superset/migrations')) + ) { + await github.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: pull.number, + body: + `@${pull.user.login} Your base branch just also updated \`superset/migrations\`.` + + 'It may cause db migration conflicts with your current changes.' + + '\n' + + '**Please consider rebasing your branch.**', + }); + break; + } + } From afa8aac538fc5252a96b60a15709c27f114faa54 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Fri, 5 Mar 2021 16:36:23 -0800 Subject: [PATCH 2/7] fix errors --- .github/workflows/check_db_migration_confict.yml | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/.github/workflows/check_db_migration_confict.yml b/.github/workflows/check_db_migration_confict.yml index 68d3a19c17b2..8f1a1cfd04c9 100644 --- a/.github/workflows/check_db_migration_confict.yml +++ b/.github/workflows/check_db_migration_confict.yml @@ -11,37 +11,39 @@ jobs: steps: - name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )" uses: actions/checkout@v2 - with: - persist-credentials: false - submodules: recursive - name: Check and notify uses: actions/github-script@v3 with: - githubToken: ${{ secrets.GITHUB_TOKEN }} + github-token: ${{ secrets.GITHUB_TOKEN }} script: | // API reference: https://octokit.github.io/rest.js + const currentBranch = context.ref.replace('ref/heads', ''); // Find all pull requests to current branch const opts = github.pulls.list.endpoint.merge({ owner: context.repo.owner, repo: context.repo.repo, - base: contect.ref, + base: currentBranch, state: 'open', sort: 'updated', per_page: 100, }); const pulls = await github.paginate(opts); + if (pulls.length > 0) { + console.log(`Found ${pulls.length} open PRs for base branch "${currentBranch}"`) + } for (const pull in pulls) { const listFilesOpts = await github.pulls.listFiles.endpoint.merge({ owner: context.repo.owner, - repo: contet.repo.repo, + repo: context.repo.repo, pull_number: pull.number, }); const files = await github.paginate(listFilesOpts); if ( files.some(x => x.contents_url.includes('/contents/superset/migrations')) ) { + console.log(`Found open PR #${pull.number} that has also added db migration`) await github.issues.createComment({ issue_number: context.issue.number, owner: context.repo.owner, @@ -53,6 +55,5 @@ jobs: '\n' + '**Please consider rebasing your branch.**', }); - break; } } From b5f0cc97ff041d6bc6b82b36a158d13743f31a98 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Fri, 5 Mar 2021 16:56:48 -0800 Subject: [PATCH 3/7] Fix error --- .github/workflows/check_db_migration_confict.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/check_db_migration_confict.yml b/.github/workflows/check_db_migration_confict.yml index 8f1a1cfd04c9..f0ba18d19536 100644 --- a/.github/workflows/check_db_migration_confict.yml +++ b/.github/workflows/check_db_migration_confict.yml @@ -6,7 +6,7 @@ on: jobs: check_db_migration_conflict: - name: Check potential DB migration conflict + name: Check DB migration conflict runs-on: ubuntu-20.04 steps: - name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )" @@ -17,13 +17,13 @@ jobs: github-token: ${{ secrets.GITHUB_TOKEN }} script: | // API reference: https://octokit.github.io/rest.js - const currentBranch = context.ref.replace('ref/heads', ''); + const currentBranch = context.ref.replace('refs/heads/', ''); // Find all pull requests to current branch const opts = github.pulls.list.endpoint.merge({ owner: context.repo.owner, repo: context.repo.repo, - base: currentBranch, + base: context.ref, state: 'open', sort: 'updated', per_page: 100, @@ -34,6 +34,7 @@ jobs: } for (const pull in pulls) { + console.log(pull); const listFilesOpts = await github.pulls.listFiles.endpoint.merge({ owner: context.repo.owner, repo: context.repo.repo, @@ -43,7 +44,7 @@ jobs: if ( files.some(x => x.contents_url.includes('/contents/superset/migrations')) ) { - console.log(`Found open PR #${pull.number} that has also added db migration`) + console.log(`PR #${pull.number} "${pull.title}" also added db migration`) await github.issues.createComment({ issue_number: context.issue.number, owner: context.repo.owner, From e7710523711886ac4141f2a5ad665d4cd89af521 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Fri, 5 Mar 2021 17:00:25 -0800 Subject: [PATCH 4/7] fix errors --- .github/workflows/check_db_migration_confict.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/check_db_migration_confict.yml b/.github/workflows/check_db_migration_confict.yml index f0ba18d19536..4e8f60346168 100644 --- a/.github/workflows/check_db_migration_confict.yml +++ b/.github/workflows/check_db_migration_confict.yml @@ -33,8 +33,7 @@ jobs: console.log(`Found ${pulls.length} open PRs for base branch "${currentBranch}"`) } - for (const pull in pulls) { - console.log(pull); + for (const pull of pulls) { const listFilesOpts = await github.pulls.listFiles.endpoint.merge({ owner: context.repo.owner, repo: context.repo.repo, From 04788e16e8406794bb0456578579a9bc5cc15e90 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Fri, 5 Mar 2021 17:08:15 -0800 Subject: [PATCH 5/7] Add emojis --- .github/workflows/check_db_migration_confict.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/check_db_migration_confict.yml b/.github/workflows/check_db_migration_confict.yml index 4e8f60346168..da8d08d4abf8 100644 --- a/.github/workflows/check_db_migration_confict.yml +++ b/.github/workflows/check_db_migration_confict.yml @@ -50,10 +50,11 @@ jobs: repo: context.repo.repo, issue_number: pull.number, body: - `@${pull.user.login} Your base branch just also updated \`superset/migrations\`.` + - 'It may cause db migration conflicts with your current changes.' + + `⚠️ @${pull.user.login} Your base branch just also updated \`superset/migrations\`.\n` + '\n' + - '**Please consider rebasing your branch.**', + 'It may cause db migration conflicts with your current changes.\n' + + '\n' + + '**❗ Please consider rebasing your branch.**', }); } } From 4efe7a665e0aeb6fd022a54ddb00394303290dc5 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Fri, 5 Mar 2021 17:16:24 -0800 Subject: [PATCH 6/7] Reformat --- .github/workflows/check_db_migration_confict.yml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/check_db_migration_confict.yml b/.github/workflows/check_db_migration_confict.yml index da8d08d4abf8..756ab87d4ddc 100644 --- a/.github/workflows/check_db_migration_confict.yml +++ b/.github/workflows/check_db_migration_confict.yml @@ -50,11 +50,10 @@ jobs: repo: context.repo.repo, issue_number: pull.number, body: - `⚠️ @${pull.user.login} Your base branch just also updated \`superset/migrations\`.\n` + + `⚠️ @${pull.user.login} Your base branch \`${currentBranch}\` has just ` + + 'also updated `superset/migrations`.\n' + '\n' + - 'It may cause db migration conflicts with your current changes.\n' + - '\n' + - '**❗ Please consider rebasing your branch.**', + '❗ **Please consider rebasing your branch to avoid db migration conflicts.**', }); } } From 845726af0f7d84e9b063e43a92bd295dffa2f1c5 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Fri, 5 Mar 2021 17:47:18 -0800 Subject: [PATCH 7/7] Update notes on db migration conflicts --- CONTRIBUTING.md | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1138e1b82a4c..0f3482ca3fcb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1001,7 +1001,7 @@ Submissions will be considered for submission (or removal) on a case-by-case bas When two DB migrations collide, you'll get an error message like this one: -``` +```text alembic.util.exc.CommandError: Multiple head revisions are present for given argument 'head'; please specify a specific target revision, '@head' to narrow to a specific head, @@ -1016,15 +1016,46 @@ To fix it: superset db heads ``` - This should list two or more migration hashes. + This should list two or more migration hashes. E.g. + + ```bash + 1412ec1e5a7b (head) + 67da9ef1ef9c (head) + ``` + +2. Pick one of them as the parent revision, open the script for the other revision + and update `Revises` and `down_revision` to the new parent revision. E.g.: + + ```diff + --- a/67da9ef1ef9c_add_hide_left_bar_to_tabstate.py + +++ b/67da9ef1ef9c_add_hide_left_bar_to_tabstate.py + @@ -17,14 +17,14 @@ + """add hide_left_bar to tabstate + + Revision ID: 67da9ef1ef9c + -Revises: c501b7c653a3 + +Revises: 1412ec1e5a7b + Create Date: 2021-02-22 11:22:10.156942 + + """ + + # revision identifiers, used by Alembic. + revision = "67da9ef1ef9c" + -down_revision = "c501b7c653a3" + +down_revision = "1412ec1e5a7b" + + import sqlalchemy as sa + from alembic import op + ``` -1. Create a new merge migration + Alternatively you may also run `superset db merge` to create a migration script + just for merging the heads. ```bash superset db merge {HASH1} {HASH2} ``` -1. Upgrade the DB to the new checkpoint +3. Upgrade the DB to the new checkpoint ```bash superset db upgrade