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

Translation cache not cleared when installing extension with translations #5727

Closed
SahAssar opened this issue Sep 1, 2016 · 13 comments
Closed
Assignees
Labels
stale Stale issues & PRs flagged for closing

Comments

@SahAssar
Copy link
Contributor

SahAssar commented Sep 1, 2016

This only affects 3.1+ (inclusive). When installing a new extension which contains translations the cache for translations is not cleared. I have verified this myself and it was also seen by @eduardomart in bacbos/bolt-menu-editor#35.

Preferably any cache that an extension can/will change (all of them?) will be cleared on install.

@SahAssar
Copy link
Contributor Author

SahAssar commented Sep 1, 2016

Assigning to @GawainLynch since you were involved with the original PR and per https://github.com/bolt/bolt/wiki/%5BIssues-Process%5D-Assigned-issues-guidelines

@GwendolenLynch
Copy link
Contributor

I don't think this is fixable currently … hackable, but you would lose the point of having caching at all.

@SahAssar
Copy link
Contributor Author

SahAssar commented Sep 2, 2016

@GawainLynch Isn't this fixable by clearing the cache on extension install? Extension installation should be rare enough that it shouldn't screw up caching, no?

@GwendolenLynch
Copy link
Contributor

Well, there isn't "screwing up caching" as such, that would mean caching was broken itself. But dumping cache like that is the hacky crap that I'd like to avoid. We've invested a lot of effort in tearing broken design approaches out of core, needlessly putting one back in seems counter intuitive?

... also, I can all but guarantee 100% of the support calls on that "feature" would be people wondering why updates to their translations aren't working.

What we need to be doing is properly managing catalogues

@SahAssar
Copy link
Contributor Author

SahAssar commented Sep 4, 2016

Seems I misinterpreted "lose the point of having caching at all" :D

@GwendolenLynch
Copy link
Contributor

Well, that is understandable. We've historically just dumped everything in the one spot, and typically only cached things like news feeds & Twig template compiles.

In current release/3.2 we cache (based on configuration):

  • Translation calatogues (TranslationCeption Annotate $app type in front controller #1555)
  • Loaded, normalised & validated application configuration
    • config.yml, contenttypes.yml, extensions.yml, menu.yml, routing.yml & taxonomy.yml
  • Codeception run data
  • PHPUnit run data
  • Composer data used in Extend
  • Environment data
    • General (think news needs, etc)
    • Database queries (more and more)
    • Exception traces
    • HTTP requests (proxy cache)
  • Profiler data (debug toolbar database)
  • Sessions
  • Installed version hash (which when changed triggers syncing of things like assets to public)
  • Moar coming to a release near you

image

While deleting any/all of it should not affect the ability of the application to boot. However, it will affect things like performance & debugging

… at least to me, it dumping a whole cache to load a file that isn't being loaded because we have the whole layer messed up seems like cutting off one's nose to spite the face.

@SahAssar
Copy link
Contributor Author

SahAssar commented Sep 5, 2016

@GawainLynch Yeah, I was taking the "simple, stupid and overkill" approach in my thinking. But it seems to me that it will lead to some confusion for people since the ui (backend and frontend, right?) for extensions will seem broken until a cache clear, and if we can find a "smart" way to fix it it'd be awesome.

If it isn't fixable that's "workable" too, let me know and I'll write some warning in the docs and PR readme updates to the extensions that use the translations.

@navelpluisje
Copy link

@GawainLynch I understand why you would not want to remove the cache after adding an extension, but why is not all the cache thrown away if I want to delete the cache manually from the menu?

@GwendolenLynch
Copy link
Contributor

Because our current implementation breaks the interface, we've mostly gotten it back now, but still work to do… Add to that, there are a bunch of things you don't want to lose when you flush cache, like sessions — all logged in users will not be happy when they lose their work, front-end users using forms will get invalidated CSRF tokens, etc — … the list goes on.

Improvements are coming, and v4 will almost certainly use Symfony Cache over Doctrine's, which gives us a tonne more flexibility with what/when/how we cache stuff, and what when we flush it. Before that we might add some functionality to enable selective stuff to be purge, but it still opens the door to a lot of upset people who flushed their cache and pissed off a bunch of users.

Basically it is a balance between functionality and UX at the moment, what isn't flushed in the UI, should only be flushed by people who both know why, and how to do it without the UI as they understand the impact that will have on site users/visitors.

@navelpluisje
Copy link

Sounds fair enough

@stale
Copy link

stale bot commented Nov 20, 2017

This issue has been automatically marked as stale because it has not had recent activity. Maybe this issue has been fixed in a recent release, or perhaps it is not affecting a lot of people?
It will be closed if no further activity occurs, because we like to keep the issue queue concise and actual.
If you think this issue is still relevant, please let us know. Especially if you’d like to help fix the issue, either by helping us pinpointing the cause of a bug, or in implementing a fix or new feature.

@stale stale bot added the stale Stale issues & PRs flagged for closing label Nov 20, 2017
@SahAssar
Copy link
Contributor Author

IMO this is another one of those "not fixable until fixable" issues that we all know about and that will be fixed when we actually fix the underlying structure.

@GwendolenLynch
Copy link
Contributor

FWIW, I have already replaced this in the upcoming branch that will land with Flex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Stale issues & PRs flagged for closing
Projects
None yet
Development

No branches or pull requests

3 participants