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

Fix: ktlint not found error #39

Closed
wants to merge 7 commits into from

Conversation

wf9a5m75
Copy link

@wf9a5m75 wf9a5m75 commented Apr 5, 2023

problem

Screen Shot 2023-04-05 at 1 08 44 AM

This PR effort

Screen Shot 2023-04-05 at 1 11 31 AM

@wf9a5m75 wf9a5m75 requested a review from a team as a code owner April 5, 2023 08:12
@@ -6,16 +6,16 @@ export BASELINE=
export CUSTOM_RULE_PATH=

if [ "$INPUT_KTLINT_VERSION" = "latest" ]; then
curl -sSL https://api.github.com/repos/pinterest/ktlint/releases/latest --header "authorization: Bearer ${INPUT_GITHUB_TOKEN}" |
Copy link
Member

Choose a reason for hiding this comment

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

@wf9a5m75 can you explain when the issue happens? This line was introduced in #33 to solve the same failure you're mentioning. Without a token, a different rate limit is applied and if many requests are performed from the same IP, you will soon reach that limit.

Copy link
Author

@wf9a5m75 wf9a5m75 Apr 5, 2023

Choose a reason for hiding this comment

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

Oh I see. In that case, why don't you implement cache for limited time?
KtLint updates once in a while (I believe they don't update every day).
But this action checks every time.
I think once a day is enough.
(But I don't know the external action can create cache)


This error occurs you for got the backslash before line-end (\n), thus the pipe does not work.

Copy link
Member

Choose a reason for hiding this comment

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

Backslash is not needed if the line ends with pipe |. Is it always failing for you when you use ScaCap/action-ktlint?

Copy link
Author

Choose a reason for hiding this comment

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

Without backslash, always fail.

Copy link
Member

Choose a reason for hiding this comment

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

@wf9a5m75 coming back to this again, can you replicate this in some public repository so we can take a look?

@wf9a5m75
Copy link
Author

wf9a5m75 commented Jul 5, 2023

Hello again. Sorry for late response. I worked on another projects.
The current version works on pull request. That's great. I appreciate for your work.

However, even the current version does not work on push. Here is the log. Please check the Kotlin linter section.
Screenshot 2023-07-05 at 2 30 36 AM

Actions log
https://github.com/wf9a5m75/WeatherApp/actions/runs/5462654641/jobs/9942305588

- name: Kotlin linter
  # uses: wf9a5m75/action-ktlint@fix_ktlint_not_found  # <-- work on commit
  uses: ScaCap/action-ktlint@master # <-- does not work on commit
  with:
    android: true

https://github.com/wf9a5m75/WeatherApp/blob/1ecb2cc3cb3ce438923136e04dd5e65b0779c0ab/.github/workflows/kotlin-linter_on-commit.yml#L16-L20


My PR fixes this issue.
Screenshot 2023-07-05 at 2 34 55 AM

Actions log
https://github.com/wf9a5m75/WeatherApp/actions/runs/5462707377/jobs/9942429736

- name: Kotlin linter
  uses: wf9a5m75/action-ktlint@fix_ktlint_not_found  # <-- work on commit
  # uses: ScaCap/action-ktlint@master # <-- does not work on commit
  with:
    android: true

https://github.com/wf9a5m75/WeatherApp/blob/dc8125fc87ed9c9b58fc7275b00baff4d2fac770/.github/workflows/kotlin-linter_on-commit.yml#L16-L20

@ghaiszaher
Copy link
Member

@wf9a5m75 could you try to adjust your workflow with github_token: ${{ secrets.GITHUB_TOKEN }}?

      - name: Kotlin linter
        uses: wf9a5m75/action-ktlint@fix_ktlint_not_found  # <-- work on commit
        # uses: ScaCap/action-ktlint@master # <-- does not work on commit
        with:
          android: true
          github_token: ${{ secrets.GITHUB_TOKEN }}

Also see https://github.com/ScaCap/action-ktlint#github_token and https://docs.github.com/en/actions/security-guides/automatic-token-authentication#using-the-github_token-in-a-workflow

@zipizigi
Copy link
Contributor

I also found the same problem.
This problem probably doesn't happen on public github.

It happens on Github Enterprise.

curl -sSL https://api.github.com/repos/pinterest/ktlint/releases/latest --header "authorization: Bearer ${INPUT_GITHUB_TOKEN}" |

If you are a Github enterprise user, the 'INPUT_GITHUB_TOKEN' token generated by Github Enterpris cannot be used on public Github.
Therefore authentication fails and ktlint cannot be downloaded.

In this case, if you specify the ktlint version, ktlint is downloaded and the action is available.

- name: Kotlin linter
  uses: ScaCap/action-ktlint@master 
  with:
      github_token: ${{ secrets.github_token }}
      ktlint_version: 0.50.0

@ghaiszaher
Copy link
Member

ghaiszaher commented Jul 19, 2023

I also found the same problem. This problem probably doesn't happen on public github.

It happens on Github Enterprise.

curl -sSL https://api.github.com/repos/pinterest/ktlint/releases/latest --header "authorization: Bearer ${INPUT_GITHUB_TOKEN}" |

If you are a Github enterprise user, the 'INPUT_GITHUB_TOKEN' token generated by Github Enterpris cannot be used on public Github. Therefore authentication fails and ktlint cannot be downloaded.

In this case, if you specify the ktlint version, ktlint is downloaded and the action is available.

- name: Kotlin linter
  uses: ScaCap/action-ktlint@master 
  with:
      github_token: ${{ secrets.github_token }}
      ktlint_version: 0.50.0

Thank you @zipizigi , you also made me realise that I missed the INPUT_GITHUB_TOKEN when a version is specified.

I don't have experience with GitHub Enterprise. The main purpose of adding this token is to avoid issues related to rate limiting set by GitHub. Without it, the IP might be blocked because of too many requests (the limit is very low there): https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#rate-limits-for-requests-from-personal-accounts

For unauthenticated requests, the rate limit allows for up to 60 requests per hour. Unauthenticated requests are associated with the originating IP address, and not the person making requests.

I guess we could add a separate flag anyway to instruct the action to just drop the token when downloading but you might still run in issues if many workflows run at the same time. Let me know if you have a better solution for GitHub Enterprise (e.g. another token that is not blocked).

@zipizigi
Copy link
Contributor

It is most accurate to use the api for the download URL.
However, you can use curl and awk to infer the version.
https://github.com/pinterest/ktlint/releases/latest --(redirect)--> https://github.com/pinterest/ktlint/releases/0.50.0.

Among the environment variables provided in Github Enterprise, there are no variables used for public Github authentication.

Therefore, it is better to change the method of specifying the version as above, or Github Enterprise users need to specify a version when running the action.

@ghaiszaher
Copy link
Member

It is most accurate to use the api for the download URL. However, you can use curl and awk to infer the version. https://github.com/pinterest/ktlint/releases/latest --(redirect)--> https://github.com/pinterest/ktlint/releases/0.50.0.

Among the environment variables provided in Github Enterprise, there are no variables used for public Github authentication.

Therefore, it is better to change the method of specifying the version as above, or Github Enterprise users need to specify a version when running the action.

Thanks, I'll consider it and see if it's an easy change.

@ghaiszaher
Copy link
Member

ghaiszaher commented Jul 19, 2023

It is most accurate to use the api for the download URL. However, you can use curl and awk to infer the version. https://github.com/pinterest/ktlint/releases/latest --(redirect)--> https://github.com/pinterest/ktlint/releases/0.50.0.
Among the environment variables provided in Github Enterprise, there are no variables used for public Github authentication.
Therefore, it is better to change the method of specifying the version as above, or Github Enterprise users need to specify a version when running the action.

Thanks, I'll consider it and see if it's an easy change.

Thanks to this answer: https://stackoverflow.com/a/54836319, I figured that latest can be used in the download URL instead of the version number but only if it switches places with download, so:

I'll raise a separate PR.

@ghaiszaher
Copy link
Member

Here we go: #45

@ghaiszaher
Copy link
Member

@zipizigi @wf9a5m75 please open an issue if the problem persists

@ghaiszaher ghaiszaher closed this Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants