-
Notifications
You must be signed in to change notification settings - Fork 0
fix: Sync Workflow Lives in Chronon #743
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
WalkthroughThis change removes the existing "Push To Canary" GitHub Actions workflow, which managed comprehensive multi-stage CI/CD for canary releases, and introduces a new "Push to Platform" workflow. The new workflow automates subtree synchronization from the Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant platform repo
participant chronon repo
GitHub Actions->>platform repo: On push to main, checkout repo
GitHub Actions->>GitHub Actions: Configure SSH for access
GitHub Actions->>chronon repo: Add as remote and fetch main
GitHub Actions->>platform repo: Pull chronon as subtree into /chronon
GitHub Actions->>platform repo: Commit and push changes to main
Possibly related PRs
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
SSH_PRIVATE_KEY: ${{ secrets.CHRONON_REPO_DEPLOY_KEY }} | ||
run: | | ||
mkdir -p ~/.ssh | ||
echo "$SSH_PRIVATE_KEY" > ~/.ssh/id_rsa |
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.
is this going to show the private key in the CI logs? Or is it possible a simple followup change could expose that?
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.
Here is it running: https://github.com/zipline-ai/platform/actions/runs/14843383331/job/41671413678 It doesn't expose the secret data.
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.
I confirmed here that a simple change won't expose the secret SSH key: https://github.com/zipline-ai/chronon/actions/runs/14872732013/job/41764246277?pr=743. Github is good about keeping secrets from getting exposed.
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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/push_to_platform.yaml (1)
29-31
: Ensure SSH key isn’t exposed. Verify thatSSH_PRIVATE_KEY
isn’t printed in logs.
🧹 Nitpick comments (3)
.github/workflows/push_to_platform.yaml (3)
1-7
: Triggers on main ✅; add concurrency. Workflow fires on pushes tomain
. Add aconcurrency
block to serialize runs and avoid overlapping subtree pushes.
54-55
: Push always runs; consider no-op skip. Skipgit push
when there are no subtree changes to reduce noise.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 55-55: no new line character at the end of file
(new-line-at-end-of-file)
32-32
: Trim trailing spaces & add EOF newline. Lines 32,35,39,47,50 have trailing spaces; line 55 needs a newline to satisfy YAML lint.Also applies to: 35-35, 39-39, 47-47, 50-50, 55-55
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 32-32: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/push_to_canary.yaml
(0 hunks).github/workflows/push_to_platform.yaml
(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/push_to_canary.yaml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/push_to_platform.yaml
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 39-39: trailing spaces
(trailing-spaces)
[error] 47-47: trailing spaces
(trailing-spaces)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 55-55: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (4)
.github/workflows/push_to_platform.yaml (4)
13-20
: Checkout ofplatform
repo is correct. Usingactions/checkout@v4
withfetch-depth: 0
ensures full history for subtree.
21-25
: Git user/email config is fine. Commits will be attributed to GitHub Actions.
32-38
: SSH key perms & agent setup follow best practices. File permission and agent steps look good.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
49-53
: Subtree pull logic is solid.git remote add … || true
prevents errors on reruns.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 50-50: trailing spaces
(trailing-spaces)
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.
Actionable comments posted: 0
🧹 Nitpick comments (5)
.github/workflows/push_to_platform.yaml (5)
14-20
: Disable token persistence Addpersist-credentials: false
to the checkout step when using SSH to avoid injecting the GITHUB_TOKEN.
33-33
: Remove trailing spaces Eliminate unnecessary spaces on blank lines to satisfy YAML lint.Also applies to: 36-36, 40-40, 48-48, 51-51
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 33-33: trailing spaces
(trailing-spaces)
56-56
: Add newline at EOF POSIX requires a trailing newline at end of file.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 56-56: no new line character at the end of file
(new-line-at-end-of-file)
50-50
: Improve remote idempotency Use a conditional check andgit remote set-url
instead of|| true
to avoid silent failures.
1-56
: Add tests/docs No tests or documentation were updated for this workflow. Consider adding an integration test or updating the README.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
[error] 51-51: trailing spaces
(trailing-spaces)
[error] 56-56: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
.github/workflows/push_to_platform.yaml
(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/push_to_platform.yaml
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
[error] 51-51: trailing spaces
(trailing-spaces)
[error] 56-56: no new line character at the end of file
(new-line-at-end-of-file)
…wy/copy_to_platform
…wy/copy_to_platform
…wy/copy_to_platform
## Summary My strategy to use a reuseble workflow doesn't work anymore because a private workflow isn't accessible from a public repo. Instead of triggering the sync, this simply runs it from here. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Chores** - Removed the previous canary release workflow, including automated build, test, and artifact deployment steps for AWS and GCP. - Introduced a new workflow to automate synchronization of code from the chronon repository into the platform repository via subtree pull and push operations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary My strategy to use a reuseble workflow doesn't work anymore because a private workflow isn't accessible from a public repo. Instead of triggering the sync, this simply runs it from here. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Chores** - Removed the previous canary release workflow, including automated build, test, and artifact deployment steps for AWS and GCP. - Introduced a new workflow to automate synchronization of code from the chronon repository into the platform repository via subtree pull and push operations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary My strategy to use a reuseble workflow doesn't work anymore because a private workflow isn't accessible from a public repo. Instead of triggering the sync, this simply runs it from here. ## Cheour clientslist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Chores** - Removed the previous canary release workflow, including automated build, test, and artifact deployment steps for AWS and GCP. - Introduced a new workflow to automate synchronization of code from the chronon repository into the platform repository via subtree pull and push operations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
My strategy to use a reuseble workflow doesn't work anymore because a private workflow isn't accessible from a public repo. Instead of triggering the sync, this simply runs it from here.
Checklist
Summary by CodeRabbit