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

DM-47507: Add workflow for triggering ConsDB migrations #278

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JeremyMcCormick
Copy link
Collaborator

@JeremyMcCormick JeremyMcCormick commented Nov 13, 2024

This PR adds a workflow which triggers automatic creation of migrations scripts in the consdb repo using repository dispatch. It activates on closed and merged PRs on which there were changes to cdb_*.yaml schemas requiring migrations to be generated. This is done using felis diff --comparator alembic so that changes which do not result in updates to the logical data model in the database will be ignored and not dispatched to consdb.

Do not merge until lsst/felis#109 is merged.

@JeremyMcCormick JeremyMcCormick marked this pull request as draft November 13, 2024 08:02
@JeremyMcCormick JeremyMcCormick force-pushed the tickets/DM-47507 branch 2 times, most recently from 953f23d to 781f744 Compare November 13, 2024 16:05
@JeremyMcCormick JeremyMcCormick changed the title DM-47507: Add initial version of cdb migration trigger workflow DM-47507: Add workflow for triggering migrations in external repos Nov 15, 2024
@JeremyMcCormick JeremyMcCormick marked this pull request as ready for review November 16, 2024 08:08
@JeremyMcCormick JeremyMcCormick force-pushed the tickets/DM-47507 branch 2 times, most recently from 5bb4ef0 to f7e6172 Compare November 16, 2024 15:39
ktlim
ktlim previously approved these changes Nov 16, 2024

on:
pull_request:
types: [closed]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just do merges to main?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do not know how to get the name of the branch that is being merged if it triggers on pushes to main. As far as I can tell, only the commit SHA is available in that case. From the PR trigger, this is easily accessible information. (If you have a suggestion on how to do this, I can incorporate it.)

.github/workflows/migrate.yaml Outdated Show resolved Hide resolved
.github/workflows/migrate.yaml Outdated Show resolved Hide resolved
.github/workflows/migrate.yaml Outdated Show resolved Hide resolved
- name: Trigger migration workflow in consdb repository
run: |
curl -X POST \
-H "Authorization: token ${{ secrets.REPO_DISPATCH_TOKEN }}" \
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a service account rubinobs-dm-admin that can be used to create this token.

.github/workflows/migrate.yaml Outdated Show resolved Hide resolved
.github/workflows/migrate.yaml Outdated Show resolved Hide resolved
@JeremyMcCormick JeremyMcCormick marked this pull request as draft November 25, 2024 21:14
@JeremyMcCormick JeremyMcCormick changed the title DM-47507: Add workflow for triggering migrations in external repos DM-47507: Add workflow for triggering ConsDB migrations Nov 25, 2024
@JeremyMcCormick JeremyMcCormick marked this pull request as ready for review November 25, 2024 21:25
@JeremyMcCormick JeremyMcCormick marked this pull request as draft November 25, 2024 21:25
@JeremyMcCormick JeremyMcCormick force-pushed the tickets/DM-47507 branch 3 times, most recently from 7cf58f1 to fc84b92 Compare November 25, 2024 22:50
@JeremyMcCormick JeremyMcCormick marked this pull request as ready for review December 2, 2024 16:59
@JeremyMcCormick JeremyMcCormick dismissed ktlim’s stale review December 4, 2024 22:27

This workflow is not functioning properly right now and needs to be re-reviewed after changes.

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.

2 participants