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

[CI] Update golangci linter step to avoid timeout #1082

Merged
merged 11 commits into from
Jun 27, 2023

Conversation

uri-weisman
Copy link
Contributor

Summary of your changes

Multiple builds are failing due to golangci linter timeout, instead of increasing the timeout let's try and add cache to the linter step, it might reduce the execution time.

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary README/documentation (if appropriate)

@uri-weisman uri-weisman requested a review from a team as a code owner June 26, 2023 06:38
@mergify
Copy link

mergify bot commented Jun 26, 2023

This pull request does not have a backport label. Could you fix it @uri-weisman? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit
    NOTE: backport-skip has been added to this pull request.

@github-actions
Copy link

github-actions bot commented Jun 26, 2023

📊 Allure Report - 💚 No failures were reported.

Result Count
🟥 Failed 0
🟩 Passed 33
⬜ Skipped 1

@@ -122,7 +122,9 @@ jobs:
uses: golangci/golangci-lint-action@v3.6.0
with:
version: latest
args: --timeout=10m --whole-files
args: --timeout=10m --whole-files -v
skip-pkg-cache: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this disable caching?

We might want to instead try running this job with the official instructions which claim that caching is enabled by default:

jobs:
  golangci:
    name: lint
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - uses: actions/setup-go@v4
        with:
          go-version: '1.20'
          cache: false
      - name: golangci-lint
        uses: golangci/golangci-lint-action@v3
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added skip-pkg-cache to address a similar issue to this one.
I'll try your suggested setup as well, however, I'm still experimenting so I'll make it a draft pr.

@uri-weisman uri-weisman marked this pull request as draft June 26, 2023 08:50
@uri-weisman uri-weisman marked this pull request as ready for review June 26, 2023 15:01
@uri-weisman uri-weisman changed the title [CI] Golang linter to store cache [CI] Update golangci linter step to avoid timeout Jun 26, 2023
Comment on lines +113 to +117
- name: Set up Go
uses: actions/setup-go@v4
with:
go-version-file: .go-version
cache: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like this would remove the caching, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

Caching is supposed to be handled in https://github.com/golangci/golangci-lint-action

Copy link
Contributor Author

@uri-weisman uri-weisman Jun 27, 2023

Choose a reason for hiding this comment

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

Yeah, they mention it in the performance section:
https://github.com/golangci/golangci-lint-action#performance

According to the logs, it seems that cache is being used by the linter:
Restored cache for golangci-lint from key 'golangci-lint.cache-2790-1b6dbac67386be50dbf356cfb1597004c1acd945' in 53191ms

As evidence, the linter run was very fast (~32s) compared to the past.

@uri-weisman uri-weisman merged commit 74b95d7 into elastic:main Jun 27, 2023
@uri-weisman uri-weisman deleted the golangci-cache branch June 27, 2023 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants