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 cache is not used when a new PR is created #81

Closed
gatheluck opened this issue Nov 10, 2022 · 8 comments · Fixed by #104
Closed

CI cache is not used when a new PR is created #81

gatheluck opened this issue Nov 10, 2022 · 8 comments · Fixed by #104
Assignees
Labels
bug Something isn't working

Comments

@gatheluck
Copy link
Contributor

Why

In #71, cache for CI is introduced. However, current cache is not used when new PR is created. This is because the current official cache action only able to use "caches belonging to the same branch name" and "caches belonging to the default branch".

At same time currently, image registry cache is not used. This will be useful when multiple containers are used in CI. However it looks rare case. So I'd like to remove image registry cache to same time, and add it to README.md as example.

Definition of Done

  • Cache is successfully used when new PR is created.
  • Image registry cache is removed and added to README.md as example.

How

  • Makes CI also runs when pushed to the default branch.
  • Remove image registry cache part.
@gatheluck gatheluck self-assigned this Nov 10, 2022
@gatheluck gatheluck added the bug Something isn't working label Nov 10, 2022
@gatheluck
Copy link
Contributor Author

gatheluck commented Nov 10, 2022

@YoshikiKubotani Does this sound reasonable to you? Especially I want to hear your option about my assumption that use of image registry cache it rare case.

@YoshikiKubotani
Copy link
Collaborator

YoshikiKubotani commented Nov 11, 2022

@gatheluck
As far as I see from the second link above, the cache action searches both branches that are included by the pull request.
So would it really be out of search if we create a new PR? I am unsure about where the caches are stored (so I might be saying a weird thing), but wouldn't it work fine if there is a cache key in the develop branch?

  1. Key npm-feature-d5ea0750 in the feature branch
  2. Key npm-feature- in the feature branch
  3. Key npm- in the feature branch
  4. Key npm-feature-d5ea0750 in the main branch
  5. Key npm-feature- in the main branch
  6. Key npm- in the main branch

@gatheluck
Copy link
Contributor Author

gatheluck commented Nov 11, 2022

Thank you for your comment!

As far as I see from the second link above, the cache action searches both branches that are included by the pull request.

Yes, but when brand new PR is created, basically there is no cache linked to the branch. So the first time you run CI by new PR, you cannot use cache. I faced this issue multiple times and find this issue report: https://stackoverflow.com/questions/69365200/github-actions-how-to-cache-dependencies-between-workflow-runs-of-different-bra

wouldn't it work fine if there is a cache key in the develop branch?

Unfortunately this is no. Because Cache scopes works first, then inside of the scope cache is searched based on cache key. So "caches belonging to the same branch name" and "caches belonging to the default branch" are only target of cache search.

@gatheluck
Copy link
Contributor Author

Actually I tested these only my forked Ascender branch. When new PR is created first CI run tool long time because there is no cache hit.

@gatheluck
Copy link
Contributor Author

@YoshikiKubotani
Copy link
Collaborator

Sure.
To sum up,

  • We want to use a cache when raising a PR for merging a new branch with 'develop'
  • But it cannot be possible because
    • the cache action only covers the default branch (i.e. in many cases 'main' or 'master') and the branch where that workflow is called, and
    • both branches do not have a cache in our case as the former is not relevant to the PR and the latter is the newly created branch.
  • So we should create a cache in the default branch as well for the cache action to be able to find the cache when the CI runs.

Is my understanding correct?

@YoshikiKubotani
Copy link
Collaborator

As for the image registry cache, I also believe we do not often face the situation of using multiple containers in the research context.

@gatheluck
Copy link
Contributor Author

We want to use a cache when raising a PR for merging a new branch with 'develop'
But it cannot be possible because
the cache action only covers the default branch (i.e. in many cases 'main' or 'master') and the branch where that workflow is called, and
both branches do not have a cache in our case as the former is not relevant to the PR and the latter is the newly created branch.
So we should create a cache in the default branch as well for the cache action to be able to find the cache when the CI runs.
Is my understanding correct?

Thank you! This is correct!

As for the image registry cache, I also believe we do not often face the situation of using multiple containers in the research context.

Thank you so much! Then I start working on this ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants