8000 [project-base] improved clearing cache behavior by grossmannmartin · Pull Request #1820 · shopsys/shopsys · GitHub
[go: up one dir, main page]
Skip to content

[project-base] improved clearing cache behavior #1820

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 11, 2020
Merged

[project-base] improved clearing cache behavior #1820

merged 1 commit into from
May 11, 2020

Conversation

grossmannmartin
Copy link
Member
@grossmannmartin grossmannmartin commented Apr 30, 2020 •
edited
Loading
Q A
Description, reason for the PR Cache is now cleared before any other commands in composer scripts and does not rely on working application. Built Docker image does not contain cache generated on build.
New feature No
BC breaks No
Fixes issues #1002
Have you read and signed our License Agreement for contributions? Yes

Note: why it's necessary to clear the cache first?
For example, after #1606 it's necessary to clear cache after composer update. But if any symfony command is run before clearing cache, application try to use old cache with incompatible method definition and fails on Declaration of Redis_ca5fc0f::multi() should be compatible with Redis::multi($mode = NULL) in /var/www/html/project-base/var/cache/prod/ContainerWnR8ZiQ/Redis_ca5fc0f.php

Note2: In Linux Kernel 3.10 with XFS filesystem and Overlay storage, this change results in an un-buildable image due to error in Linux kernel. (see moby/moby#10294 (comment))

@grossmannmartin grossmannmartin force-pushed the mg-clear-cache-improvement branch from 7db52d2 to e58ce48 Compare April 30, 2020 21:26
@grossmannmartin grossmannmartin marked this pull request as draft April 30, 2020 22:08
@grossmannmartin grossmannmartin force-pushed the mg-clear-cache-improvement branch from 2602093 to 217e383 Compare May 1, 2020 09:09
@pk16011990
Copy link
Member

Why do not we clear cachce by rm -rf (or multiplatform equivalent in phing)? It can cause still problems during development after switch between branches, because it is chicken egg problem (see symfony/symfony#23685)

@grossmannmartin grossmannmartin force-pushed the mg-clear-cache-improvement branch from 8b122d9 to dc07c6f Compare May 1, 2020 12:48
@grossmannmartin
Copy link
Member Author

From what I found and tested it's pretty hard to get into a state in which clear:cache command fails (see symfony/symfony#23792 – rebootable kernel)

Anyway even with clear:cache --no-warmup or php phing clean as a first script I'm still getting errors on our CI while building docker image (travis is ok, local builds too) [stage CI, step php phing composer-dev ...]

!!
#25 6.816 !!  In AbstractTypeGenerator.php line 381:
#25 6.816 !!
#25 6.816 !!    Warning: mkdir(): Invalid argument

@sonarqubecloud
Copy link
sonarqubecloud bot commented May 1, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

henzigo
henzigo reviewed May 2, 2020
View reviewed changes
@grossmannmartin grossmannmartin force-pushed the mg-clear-cache-improvement branch from 460cb0b to ecd77eb Compare May 6, 2020 08:34
@sonarqubecloud
Copy link
sonarqubecloud bot commented May 6, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@grossmannmartin grossmannmartin mentioned this pull request May 6, 2020
Closed
@grossmannmartin grossmannmartin marked this pull request as ready for review May 6, 2020 08:58
@grossmannmartin
Copy link
Member Author

Why do not we clear cache by rm -rf (or multiplatform equivalent in phing)?

I end up using php phing clean.

@grossmannmartin grossmannmartin merged commit 2e35455 into master May 11, 2020
@grossmannmartin grossmannmartin deleted the mg-clear-cache-improvement branch May 11, 2020 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@henzigo henzigo henzigo left review comments

@pesektomas pesektomas pesektomas approved these changes

@sslt sslt sslt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@grossmannmartin @pk16011990 @henzigo @pesektomas @sslt
0