Skip to content

Mandychen/migrate buildkite to GitHub actions #6946

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

Open
wants to merge 65 commits into
base: master
Choose a base branch
from

Conversation

mandyschen
Copy link

What changed?
Added Github Actions implementation to buildkite pipeline pipeline-master.yml

Why?
To keep all the pipelines in the same place

How did you test it?
Ran the pipeline through my own commits. Plan to test on different commits and compare outcomes with prior buildkite pipeline

Potential risks
Some issues aren't caught because they weren't caught in the pipeline. Some good PRs cant be merged because they were falsely caught by the pipeline

Release notes

Documentation Changes

Comment on lines 18 to 19
# - name: Check IDL submodule status (must point to master)
# run: make .idl-status # TODO: add this back in
Copy link
Member

Choose a reason for hiding this comment

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

please uncomment this and double check its output once PR is merged

@@ -0,0 +1,13 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

where do we use this file?

Copy link
Author

@mandyschen mandyschen Jun 24, 2025

Choose a reason for hiding this comment

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

Deleted

@@ -0,0 +1,17 @@
#!/bin/sh
Copy link
Member

Choose a reason for hiding this comment

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

this folder is exact copy of scripts/buildkite folder. After fully deprecating buildkite we should remember to cleanup those folders. Please add # TODO(bk-cleanup): Remove this file after fully migrated to github actions comment to all such files so we don't miss them in the future.

with:
go-version: '1.23.4'

- name: Run integration profile for sqlite
Copy link
Member

Choose a reason for hiding this comment

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

interestingly, sqlite integration tests took longer than others in this run https://github.com/mandyschen/cadence/actions/runs/15742327622/job/44372636711
@arzonus any ideas why it is taking longer in github vs buildkite (25m vs 15m)?

Copy link
Contributor

@arzonus arzonus Jun 24, 2025

Choose a reason for hiding this comment

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

Agree, interesting. I'd like to have a few more attempts to see if that is permanent or just a test flakiness. Not sure if there is a difference in CPU/Memory between Github and Buildkite, as other tests looks fine.

@mandyschen could you please retry a few more times to see this? Can't do it.

Copy link
Author

Choose a reason for hiding this comment

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

yes, I can but it had to rerun after a failure so its likely it was the combination of two runs that made it 25m

Copy link
Author

Choose a reason for hiding this comment

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

Just reran, and I think it's because it fails the first time around. The screenshots show the times are about the same for attempt #2.

image

image

Copy link
Author

@mandyschen mandyschen Jun 24, 2025

Choose a reason for hiding this comment

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

Top is buildkite logs bottom is github action logs. This test in general is a bit flaky. It soft fails in the bk version (as well as the gh version) and there's a comment saying persistence tests shouldn't fail the build.

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.

4 participants