Skip to content

Conversation

@marcocastignoli
Copy link
Member

With the previous PR (#169), the r2 mirror script can no longer run because the .git files are removed. In general, both scripts run in the same environment, causing the second script to repeat the timestamp updates, removal of hidden files, and creation of symlinks that the S3 version has already performed.

With this PR, we introduce two separate GitHub actions: one for R2 and one for S3.

@marcocastignoli marcocastignoli self-assigned this Nov 27, 2025
@marcocastignoli marcocastignoli moved this to Sprint - Needs Review in Sourcify Public Nov 27, 2025
@marcocastignoli marcocastignoli changed the title Create two separate actions fro R2 and S3 upload Create two separate actions for R2 and S3 upload Nov 27, 2025
Copy link
Collaborator

@cameel cameel left a comment

Choose a reason for hiding this comment

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

Some comments need to be updated, but other than that the code seems fine, so feel free to merge whenever you're ready.

Comment on lines +31 to +36
- name: Wait for other instances of this workflow to finish
# It's not safe to run two S3 sync operations concurrently with different files
uses: softprops/turnstyle@v1
with:
same-branch-only: no
poll-interval-seconds: 120 # in seconds
Copy link
Collaborator

@cameel cameel Nov 27, 2025

Choose a reason for hiding this comment

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

For a moment I was worried that this might prevent the R2 sync from running concurrently with the S3 one. But looking at the source, the action only waits if the workflow name matches, so it should be fine. I originally expected that you were going to put both jobs in the same workflow, but if you did that, it would block, so good that you did not.

Still, after we merge, we should check to see if both are indeed running at the same time.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be good to also update the comment I asked you to add in #169 (comment) to say that the actions are not in fact idempotent and that each script will only work on a fresh checkout.

@cameel
Copy link
Collaborator

cameel commented Nov 27, 2025

As a side note, we should also pin those actions. Back when I wrote this we weren't doing that consistently, but now that's what we normally do. Looks like github even recently added an option to enforce that in PRs (Enforce SHA pinning) and we should enable it. But that's just FYI. It's been like that before and it's not strictly related to the PR, just something I noticed while reviewing it. I'll add this as an org TODO.

@marcocastignoli marcocastignoli merged commit d8ebe0b into gh-pages Dec 1, 2025
6 of 7 checks passed
@marcocastignoli marcocastignoli deleted the fix-sync-r2 branch December 1, 2025 08:42
@github-project-automation github-project-automation bot moved this from Sprint - Needs Review to Sprint - Done in Sourcify Public Dec 1, 2025
@marcocastignoli marcocastignoli moved this from Sprint - Done to COMPLETED in Sourcify Public Dec 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: COMPLETED

Development

Successfully merging this pull request may close these issues.

3 participants