-
Notifications
You must be signed in to change notification settings - Fork 3
[SCHEMATIC-342] Refactor Workflows #196
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
Conversation
70f5dda to
dc79191
Compare
This reverts commit e2ddf08.
302d921 to
1ab3a48
Compare
BryanFauble
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me! I also dropped this workflow into claude to create a flowchart of the process. It may be worthwhile to have this included in some docs somewhere if you like it:
flowchart TD
A[PR Event: synchronize/closed on main<br/>paths: modules/**] --> B{Triggering actor ≠<br/>commit-to-main-bot?}
B -->|No| END1[End - Skip workflow]
B -->|Yes| C[schema-convert Job]
C --> D[Setup: Checkout, Python 3.10, Install deps]
D --> E[List changed files for testing]
E --> F[Assemble CSV data model]
F --> G[Commit CSV changes]
G --> H[Convert CSV to JSON-LD]
H --> I[Commit JSON-LD changes]
I --> J[Identify changed manifests]
J --> K[Save changed manifests to output]
K --> L[Delay 60 seconds]
L --> M{PR Merged?}
M -->|No| N[test Job]
M -->|Yes| O[generate-and-upload-manifests Job]
N --> N1[Setup: Checkout, Python 3.10, Install deps]
N1 --> N2[Generate test manifests]
N2 --> N3[Create test suite report with Docker/R]
N3 --> N4[Add PR comment with test results]
N4 --> N5[Upload test artifacts]
N5 --> END2[End]
O --> O1[Setup: Checkout main branch, Python 3.10, Install deps]
O1 --> O2[Generate changed manifests]
O2 --> O3[Commit manifests to main]
O3 --> O4[Commit schemas to main]
O4 --> O5[Upload manifests to Synapse]
O5 --> END3[End]
style A fill:#e1f5fe
style M fill:#fff3e0
style N fill:#f3e5f5
style O fill:#e8f5e8
style END1 fill:#ffebee
style END2 fill:#f3e5f5
style END3 fill:#e8f5e8
Thanks @BryanFauble and that will be useful! |
thomasyu888
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 Fantastic work here! I'm going to defer to Bryan and/or Andrew's final review.
|
LGTM! |
Problem
The two existing workflows (
CIandstore-current-manifests) required manual intervention and it was confusing for users to understand what was required of them along the way from committing changes to the data model in a PR and seeing those changes surface on synapse in the form of update metadata manifests.Solution
Refactor the two existing workflows into one workflow file that requires less manual intervention. Documentation will be added in a later PR.
The previous workflow files have also been removed.
The new workflow (
build) is separated into 3 jobs:schema-convert,test, andgenerate-and-upload-manifests. The workflow will run when all of the following conditions are met:modulessubdirectoryschema-convertandtestwill execute when a PR is open whileschema-convertandgenerate-and-upload-manifestswill run on the development branch after the PR is closed.The associated scripts have also been updated.
Testing
Due to the nature of github workflows and the structure of the repository, more extensive development history is in PRs #198, #200, #201, #202, #203, and #204.
Final testing was performed on #201, #202, #203, and #204, with
test-mainacting as the dummymainbranch andtest-dev-branchserving as a dummy development branch.Functionality of the PR-open operations was confirmed in #202 (
buildrun #189)schema-convertfunctionality is evident from commits a9135a3 and 5d7b999testfunctionality is evident from the github actions bot comment on the pull request and the successful template generation linked in the comment.Functionality of the PR-close operations was confirmed in #203 (
buildrun #192)schema-convertfunctionality is evident from commits a9135a3 and 5d7b999generate-and-upload-manifestsfunctionality is evident from commits 112376d and 38a1476