Skip to content
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

Crowdin CI v2 #12922

Merged
merged 20 commits into from
May 31, 2024
Merged

Crowdin CI v2 #12922

merged 20 commits into from
May 31, 2024

Conversation

wackerow
Copy link
Member

@wackerow wackerow commented May 8, 2024

Description

  • Updates get-translations.ts with new scripts
  • Returns build ID from triggerBuild script
  • Adds awaitLatestBuild script, accepting the build ID from previous step
    • Checks every 10 seconds to see if build status is "finished"
    • Time out set to 2 hours
  • Fetches bucket/directory IDs before performing iport
  • Trigger coderabbitai if md warnings found

TODO:

  • Check if any other scripts or calls are redundant to see if they can be trimmed

extract env vars to single-declaration; remove sleep and replace with awaitLatestBuild script; move timing to AM,
this is updated with each import cycle and committed; large number of build minutes and not required here
@github-actions github-actions bot added needs review 👀 tooling 🔧 Changes related to tooling of the project labels May 8, 2024
Copy link

netlify bot commented May 8, 2024

Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit 0a50d17
🔍 Latest deploy log https://app.netlify.com/sites/ethereumorg/deploys/664cc0f648551f00083ebb2f
😎 Deploy Preview https://deploy-preview-12922--ethereumorg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
7 paths audited
Performance: 51 (🔴 down 5 from production)
Accessibility: 92 (no change from production)
Best Practices: 94 (🔴 down 4 from production)
SEO: 93 (🔴 down 2 from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

in lieu of incorporation into CI/CD workflow; keeps triggerBuild.ts script
Repeat calls could produce the same build ID. `import "dotenv/config"` not needed when called from an action
Requires getting process.env.BUILD_ID from previous script in workflow (triggerBuild.ts)
Comment on lines 48 to 49
OUTPUT=$(npx ts-node -O '{"module":"commonjs"}' ./src/scripts/crowdin/translations/triggerBuild.ts)
echo "$OUTPUT"
Copy link
Member Author

Choose a reason for hiding this comment

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

This syntax echo's the console.log statements made from inside triggerBuild.ts. Console log statements matching a particular pattern of ::set-output name=key_name::value can be extracted in a future step.

In triggerBuild.ts we're console logging ::set-output name=buildId::${id} which allows us to get the buildId variable next.

Comment on lines 53 to 54
env:
BUILD_ID: ${{ steps.build-crowdin.outputs.buildId}}
Copy link
Member Author

Choose a reason for hiding this comment

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

This assigns the buildId output from the previous step (step id: build-crowdin) to an environment variable to be accessed by the next script.

Comment on lines 56 to 57
OUTPUT=$(npx ts-node -O '{"module":"commonjs"}' ./src/scripts/crowdin/translations/awaitLatestBuild.ts)
echo "$OUTPUT"
Copy link
Member Author

Choose a reason for hiding this comment

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

Similar syntax to above, but this time we're outputting ::set-output name=buildSuccess::true (or false) depending if the build eventually switched to "finished" status or not.

Comment on lines 61 to 65
run: |
if [ "${{ steps.build-crowdin.outputs.buildSuccess }}" = "false" ]; then
echo "Build timed out, exiting"
exit 1
fi
Copy link
Member Author

Choose a reason for hiding this comment

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

If the buildSuccess from the prior step was "false" the we exit early due to timing out. exit 1 will be treated as a failure, and the workflow will stop here.

Comment on lines 69 to 70
- name: Get latest translation bucket directory ids
run: npx ts-node -O '{"module":"commonjs"}' ./src/scripts/crowdin/translations/getBucketDirectoryIds.ts
Copy link
Member Author

Choose a reason for hiding this comment

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

Run the script to get the up-to-date bucket directory id's

Copy link
Member Author

Choose a reason for hiding this comment

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

Created this since fetchAndSaveDirectories does not automatically call this function, and it is used elsewhere so I wanted to avoid conflicting.

Comment on lines 3 to 4
import "dotenv/config"

Copy link
Member Author

Choose a reason for hiding this comment

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

Turns out this isn't needed when running from an action

`Build ${isAlreadyFinished ? "already finished" : "triggered"} id:`,
id
)
console.log(`::set-output name=buildId::${id}`)
Copy link
Member Author

Choose a reason for hiding this comment

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

output id for use in next workflow step

}

if (data.status !== FINISHED) {
console.error(`::set-output name=buildSuccess::false`)
Copy link
Member Author

@wackerow wackerow May 9, 2024

Choose a reason for hiding this comment

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

If fetch has timed out and still not finished, export buildSuccess=false to terminate workflow early.

Copy link
Member Author

Choose a reason for hiding this comment

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

No longer needed as a dedicated workflow.

@wackerow wackerow marked this pull request as ready for review May 9, 2024 00:47
@wackerow wackerow marked this pull request as draft May 9, 2024 02:13
@wackerow
Copy link
Member Author

Closing for now in lieu of #12936

@wackerow wackerow closed this May 15, 2024
@github-actions github-actions bot added the abandoned This has been abandoned or will not be implemented label May 15, 2024
@wackerow wackerow changed the title Crowdin CD v2 Crowdin CI v2 May 18, 2024
@wackerow
Copy link
Member Author

#12936 attempted to use crowdin/github-action@v1 to simplify build/await/download process for translations, though comes with nuances and requirements that do not currently meet our needs. Re-opening this PR as preferential approach.

@wackerow wackerow reopened this May 18, 2024
@wackerow wackerow removed the abandoned This has been abandoned or will not be implemented label May 18, 2024
@wackerow
Copy link
Member Author

@wackerow wackerow marked this pull request as ready for review May 20, 2024 10:57
jobs:
create_approved_language_bucket_prs:
runs-on: ubuntu-latest
env:
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved these up to avoid re-declaring for each step.

env:
CROWDIN_API_KEY: ${{ secrets.CROWDIN_API_KEY }}
CROWDIN_PROJECT_ID: ${{ secrets.CROWDIN_PROJECT_ID }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Copy link
Member Author

Choose a reason for hiding this comment

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

By providing this token, the GitHub CLI will automatically authenticate; have removed the gh auth step.

id: build-crowdin
run: |
npx ts-node -O '{"module":"commonjs"}' ./src/scripts/crowdin/translations/triggerBuild.ts;
grep BUILD_ID output.env >> $GITHUB_ENV;
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the latest recommended way to pass a variable to a future job step. the set-output approach originally being used here has been deprecated, so I've converted to this approach.


- name: Check build success
run: |
if [ $(grep BUILD_SUCCESS output.env | cut -d'=' -f2) = false ]; then
Copy link
Member Author

Choose a reason for hiding this comment

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

BUILD_SUCCESS will be "false" if the previous step times-out without a finished build available. exit 1 will terminate the entire workflow with an error in this case.

@@ -60,6 +58,7 @@ export const scrapeDirectory = (
// Update .md tracker
trackers.langs[repoLangCode].mdCopyCount++
} else {
if (!statSync(source).isDirectory()) return
Copy link
Member Author

Choose a reason for hiding this comment

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

This else block was catching the non-markdown/JSON files (such as .xlsx) and attempting to open as a directory; this guard clause will skip non-directories.

Copy link
Contributor

@minimalsm minimalsm left a comment

Choose a reason for hiding this comment

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

LGTM :)

@wackerow wackerow merged commit 7bce095 into dev May 31, 2024
6 checks passed
@wackerow wackerow deleted the crowdin-cd-v2 branch May 31, 2024 14:44
This was referenced Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tooling 🔧 Changes related to tooling of the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants