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: fix clean-up order #22

Merged
merged 2 commits into from
Jul 10, 2023
Merged

fix: fix clean-up order #22

merged 2 commits into from
Jul 10, 2023

Conversation

ryuni-dev
Copy link
Contributor

@ryuni-dev ryuni-dev commented Jul 7, 2023

📌 Type of PR

local cache의 purge 순서를 바꾸었습니다.

♻️ Current situation

  • cache purge를 cache save 이후 action의 마지막 부분에서 합니다.
  • cache restore이 먼저 이루어지고 purge가 되기 때문에 아래 문제가 발생합니다.
    • 일주일(expiration)이 지난 캐시의 경우도 restore이 먼저됩니다.
    • 액션이 끝나고 post 액션에서 cache save가 되고, 이후 바로 purge 됩니다.
    • 이후 다른 job에서 해당 캐시를 사용하려할 때 캐시가 삭제되어서 이미 있는 줄 알고 쓰려다 실패합니다.

💡 Proposed solution

  • purge를 restore 보다 먼저하는 것으로 변경하였습니다.
  • purge 도중 익셉션 발생할 경우 액션이 멈출 수 있으니, try-catch 분리해서 따로 처리하였습니다.

⚙️ Release Notes

🧪 Testing

  • BEAT repo에서 테스팅 완료했습니다!

@ryuni-dev ryuni-dev requested a review from seoljiwon July 7, 2023 10:34
@ryuni-dev ryuni-dev self-assigned this Jul 7, 2023
src/restore/index.ts Outdated Show resolved Hide resolved
@ryuni-dev ryuni-dev requested a review from seoljiwon July 10, 2023 00:50
@seoljiwon
Copy link
Contributor

좋아요!!!! 첫 PR인가요? 축하합니다 🎉🎉🎉

@ryuni-dev
Copy link
Contributor Author

감사합니다!! :)

@ryuni-dev ryuni-dev merged commit 67dba3e into main Jul 10, 2023
3 checks passed
@ryuni-dev ryuni-dev deleted the fix/clean-up branch July 10, 2023 03:13
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.

2 participants