Skip to content

ci: fix yaml (again) #7107

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

Merged
merged 5 commits into from
Feb 9, 2023
Merged

ci: fix yaml (again) #7107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
59f4ce9
ci: fix yaml (again)
mydea Feb 9, 2023
efb2569
fix ember lint
mydea Feb 9, 2023
ddb1f21
add clear cache workflow
mydea Feb 9, 2023
0751a49
always run `yarn install`
mydea Feb 9, 2023
e04962c
revert to old build command
mydea Feb 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,10 @@ jobs:
is_develop: ${{ github.ref == 'refs/heads/develop' }}
is_release: ${{ startsWith(github.ref, 'refs/heads/release/') }}
is_gitflow_sync: ${{ github.head_ref == 'refs/heads/develop' || github.head_ref == 'refs/heads/master' }}
has_gitflow_label: ${{ github.event_name == 'pull_request' && contains(steps.pr-labels.outputs.labels, ' Gitflow ') }}
force_skip_cache: ${{ github.event_name == 'pull_request' && contains(steps.pr-labels.outputs.labels, ' ci-skip-cache ') }}
has_gitflow_label:
${{ github.event_name == 'pull_request' && contains(steps.pr-labels.outputs.labels, ' Gitflow ') }}
force_skip_cache:
${{ github.event_name == 'pull_request' && contains(steps.pr-labels.outputs.labels, ' ci-skip-cache ') }}

job_install_deps:
name: Install Dependencies
Expand Down Expand Up @@ -172,7 +174,6 @@ jobs:
key: ${{ steps.compute_lockfile_hash.outputs.hash }}

- name: Install dependencies
if: steps.cache_dependencies.outputs.cache-hit == '' || needs.job_get_metadata.outputs.force_skip_cache == 'true'
Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I don't think we need this, when we hit the cache this step will be more or less instantenous anyhow. May help here or there with an inconsistency.

run: yarn install --ignore-engines --frozen-lockfile
outputs:
dependency_cache_key: ${{ steps.compute_lockfile_hash.outputs.hash }}
Expand Down Expand Up @@ -215,7 +216,8 @@ jobs:
path: node_modules/.cache/nx
key: nx-Linux-${{ github.ref }}-${{ env.HEAD_COMMIT }}
# On develop branch, we want to _store_ the cache (so it can be used by other branches), but never _restore_ from it
restore-keys: ${{needs.job_get_metadata.outputs.is_develop == 'false' && env.NX_CACHE_RESTORE_KEYS || 'nx-never-restore'}}
restore-keys:
${{needs.job_get_metadata.outputs.is_develop == 'false' && env.NX_CACHE_RESTORE_KEYS || 'nx-never-restore'}}

- name: Build packages
# Under normal circumstances, using the git SHA as a cache key, there shouldn't ever be a cache hit on the built
Expand Down Expand Up @@ -798,7 +800,9 @@ jobs:
needs: [job_get_metadata, job_build]
runs-on: ubuntu-20.04
timeout-minutes: 30
if: contains(github.event.pull_request.labels.*.name, 'ci-overhead-measurements') || needs.job_get_metadata.outputs.is_develop == 'true'
if: |
contains(github.event.pull_request.labels.*.name, 'ci-overhead-measurements') ||
needs.job_get_metadata.outputs.is_develop == 'true'
steps:
- name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})
uses: actions/checkout@v3
Expand Down
11 changes: 11 additions & 0 deletions .github/workflows/clear-cache.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
name: Clear all GHA caches
on:
workflow_dispatch:

jobs:
clear-caches:
name: Delete all caches
runs-on: ubuntu-20.04
steps:
- name: Clear caches
uses: easimon/wipe-cache@v2
7 changes: 4 additions & 3 deletions .github/workflows/gitflow-sync-master.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
name: Gitflow - Sync develop into master
on:
pull_request:
- types: [closed]
- branches:
- 'develop'
types:
- closed
branches:
- develop

env:
MAIN_BRANCH: master
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"private": true,
"scripts": {
"build": "node ./scripts/verify-packages-versions.js && lerna run build:types,build:transpile,build:bundle",
"build": "node ./scripts/verify-packages-versions.js && run-s build:types build:transpile build:bundle",
Copy link
Member Author

Choose a reason for hiding this comment

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

cc @JamesHenry I reverted this here, as it seems that we sometimes ended up with an inconsistent state because of this (??) TBH we are not quite sure what is going on, but in multiple tests in weird ways, e.g. https://github.com/getsentry/sentry-javascript/actions/runs/4132316633/jobs/7140902414

I am not really sure that this is the actual problem, but it's my best guess so far that some internal race condition or whatever leads to something being overwritten or not properly build... Will investigate some more!

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry about that @mydea - I'll take a look.

FYI I invited you on Slack Connect, sent to your sentry email, so you can also reach me there

Copy link
Contributor

Choose a reason for hiding this comment

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

@mydea At a quick glance it looks like tests depend on builds having run already, at the very least build:types?

We can naturally extend our existing task pipelines to let the lerna task runner handle coordinating this for us. I'll put up a PR

Copy link
Contributor

@JamesHenry JamesHenry Feb 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

There are also a couple of missing relationships from the package graph right now (because the relevant package.json files are missing them)

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 worries, again I am not even really sure this was the problem, we seem to have an issue with a bunch of CI caching stuff coming together. Thanks for helping us figure this out!

I've put a request with our IT to get a shared slack channel, it is pending - looking forward to it going through!

I was also thinking about getting rid of our CI build cache and just leverage Nx cache. It would probably streamline stuff quite a bit!

Copy link
Contributor

Choose a reason for hiding this comment

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

I've put a request with our IT to get a shared slack channel, it is pending - looking forward to it going through!

Ah ok - I had sent the Slack Connect invite from our side your sentry email - did you receive it? Did you forward it on to your IT, or did you initialize the process separately from your side?

I was also thinking about getting rid of our CI build cache and just leverage Nx cache. It would probably streamline stuff quite a bit!

💯 and I can help set this up for you. As an OSS project you get unlimited remote caching for free

Copy link
Contributor

Choose a reason for hiding this comment

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

Cacheable tests PR which should hopefully resolve the issues you've been seeing as well: #7108

"build:bundle": "lerna run build:bundle",
"build:dev": "lerna run build:types,build:transpile",
"build:dev:filter": "lerna run build:dev --include-filtered-dependencies --include-filtered-dependents --scope",
Expand Down
2 changes: 1 addition & 1 deletion packages/ember/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"clean": "yarn rimraf sentry-ember-*.tgz dist tmp build .node_modules.ember-try package.json.ember-try",
"lint": "run-p lint:js lint:hbs lint:ts",
"lint:hbs": "ember-template-lint .",
"lint:js": "eslint . --cache --cache-location '../../eslintcache/'",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we removed all of these in a previous PR, just forgot about this. (Stuff I find when trying to debug stuff locally)

"lint:js": "eslint .",
"lint:ts": "tsc",
"start": "ember serve",
"test": "ember test",
Expand Down