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

feat: add caching for testkube binary #4

Merged
merged 4 commits into from
Jan 26, 2024
Merged

Conversation

devcatalin
Copy link
Member

@devcatalin devcatalin commented Jan 25, 2024

This PR...

Changes

Fixes

Checklist

  • tested locally
  • added new dependencies
  • updated the docs
  • added a test

src/index.ts Outdated Show resolved Hide resolved
@devcatalin devcatalin requested a review from rangoo94 January 25, 2024 15:23
Copy link
Member

@rangoo94 rangoo94 left a comment

Choose a reason for hiding this comment

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

Code LGTM, please test if the action works as expected though 👍

src/index.ts Outdated Show resolved Hide resolved
@itsme2980
Copy link

Hi @devcatalin , @rangoo94 Thank you for the concise support regarding this feature. I've recently had the opportunity to review your implementation, and I'd like to share some comments from my perspective. This approach is only viable once Testkube is successfully installed within the GitHub Actions (GA) workflow. This means the cache is effective only when Testkube has been installed. There's a scenario where GitHub.io might impose rate limits, rejecting installation from anonymous access, rendering this caching implementation ineffective.

I propose we implement an additional feature: allowing users to define the GitHub.io Personal Access Token (PAT) within the action.yaml. This PAT could then be used to authorize with GitHub.io, overcoming potential rate-limiting issues. For more details about the token property I mentioned, you can refer to https://github.com/actions/setup-python.

The combination of a GitHub.io token and caching would be highly beneficial. 👍

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.

3 participants