Skip to content

(fixes:#19994)prevent redundant translation sync runs and enforce 30-minute delay #20004

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.

Sign up for GitHub

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

Jump to bottom
Closed
gurnoorpannu wants to merge 1 commit into from
Closed

(fixes:#19994)prevent redundant translation sync runs and enforce 30-minute delay #20004

gurnoorpannu wants to merge 1 commit into from
+45 −3

Conversation

@gurnoorpannu
Copy link
Contributor

@gurnoorpannu gurnoorpannu commented Jan 4, 2026

Purpose / Description

This PR adds two key improvements:-

  1. Concurrency Control
  2. 30-Minute Delay

Fixes

Approach

Updated timeout-minutes from 20 to 50 to accommodate the delay
Fixed action versions: actions/checkout@v4 and actions/setup-node@v4

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration (SDK version(s), emulator or physical, etc)

Learning (optional, can help others)

Describe the research stage

Links to blog posts, patterns, libraries or addons used to solve this problem

Checklist

Please, go through these checks before submitting the PR.

  • [ Y] You have a descriptive commit message with a short title (first line, max 50 chars).
  • [ Y] You have commented your code, particularly in hard-to-understand areas
  • [ Y] You have performed a self-review of your own code
  • [NA ] UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • [ NA] UI Changes: You have tested your change using the Google Accessibility Scanner

@gurnoorpannu gurnoorpannu changed the title CI: prevent redundant translation sync runs and enforce 30-minute delay (fixes:#19994)prevent redundant translation sync runs and enforce 30-minute delay Jan 4, 2026
david-allison
david-allison requested changes Jan 4, 2026
View reviewed changes
Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

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

This doesn't wait 30 minutes after the last call, this waits an additional 30 minutes.

@gurnoorpannu
Copy link
Contributor Author

gurnoorpannu commented Jan 4, 2026

This doesn't wait 30 minutes after the last call, this waits an additional 30 minutes.

oh! you are right,I’ll update the workflow to calculate the remaining wait time based on the last run and only delay when necessary.

@david-allison david-allison added the Needs Author Reply Waiting for a reply from the original author label Jan 4, 2026
sanjaysargam
sanjaysargam suggested changes Jan 5, 2026
View reviewed changes
Copy link
Member

@sanjaysargam sanjaysargam left a comment

Choose a reason for hiding this comment

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

squash commits into one

.github/workflows/sync_translations.yml
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v6
- uses: actions/checkout@v4
Copy link
Member

@sanjaysargam sanjaysargam Jan 5, 2026

Choose a reason for hiding this comment

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

why?

.github/workflows/sync_translations.yml
shell: bash

- uses: actions/setup-node@v6
- uses: actions/setup-node@v4
Copy link
Member

@sanjaysargam sanjaysargam Jan 5, 2026

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

@gurnoorpannu gurnoorpannu Jan 5, 2026

Choose a reason for hiding this comment

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

these were changed because actions/checkout@v6 and actions/setup-node@v6 don’t exist and would fail at runtime. I switched them to @v4, which is the latest stable and supported version

Copy link
Member

@david-allison david-allison Jan 5, 2026

Choose a reason for hiding this comment

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

these were changed because actions/checkout@v6 and actions/setup-node@v6 don’t exist and would fail at runtime.

? source

Copy link
Contributor Author

@gurnoorpannu gurnoorpannu Jan 6, 2026

Choose a reason for hiding this comment

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

these were changed because actions/checkout@v6 and actions/setup-node@v6 don’t exist and would fail at runtime.

? source

https://github.com/actions/checkout/releases
https://github.com/actions/setup-node/releases

There is no published v6 release for either action, so using @v6 would fail at runtime. I switched them to @v4, which is the latest supported version.

Copy link
Member

@david-allison david-allison Jan 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

They exist?

https://github.com/actions/checkout/releases/tag/v6.0.0
https://github.com/actions/setup-node/releases/tag/v6.0.0

In addition: CI was working before this change using v6

This was done by dependabot:

@gurnoorpannu
CI: prevent redundant translation sync runs and enforce 30-minute gap
9f8b36d 
- Add concurrency control to prevent overlapping runs
- Cancel in-progress runs when new one is triggered
- Enforce minimum 30-minute gap between syncs (not fixed delay)
- Check last commit time on i18n_sync branch
- Calculate remaining wait time dynamically
- Proceed immediately if >= 30 minutes have passed
- Add defensive guard against negative sleep values
- Improve log clarity by showing elapsed time in seconds
- Update timeout from 20 to 50 minutes
- Fix action versions to v4 (checkout and setup-node)
@gurnoorpannu gurnoorpannu force-pushed the fix-translation-sync-concurrency-clean branch from 1e3561d to 9f8b36d Compare January 6, 2026 11:39
@david-allison
Copy link
Member

david-allison commented Jan 6, 2026

I don't have confidence in this change given #20004 (comment)

@gurnoorpannu
Copy link
Contributor Author

gurnoorpannu commented Jan 7, 2026

I don't have confidence in this change given #20004 (comment)

no worries if you think so I can revert these changes back!

@gurnoorpannu gurnoorpannu mentioned this pull request Jan 7, 2026
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@david-allison david-allison david-allison requested changes

+1 more reviewer

@sanjaysargam sanjaysargam sanjaysargam requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Needs Author Reply Waiting for a reply from the original author

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Schedule Sync translations action to run 30 minutes after the last one.

3 participants

@gurnoorpannu @david-allison @sanjaysargam