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

Remove all instances of reading/writing that does not use FlySystem #4977

Open
14 tasks
Mnkras opened this issue Jan 19, 2017 · 4 comments
Open
14 tasks

Remove all instances of reading/writing that does not use FlySystem #4977

Mnkras opened this issue Jan 19, 2017 · 4 comments
Labels
Affects:Developers Affects those who customize the CMS using code. Enhancement:Level Of Effort:3 Enhancements requiring a significant level of effort. Enhancement:Quality Of Life:1 Enhancements that will have a negligible or small improvement in quality of life. Product Areas:Code Quality Product Areas:Framework Status:Available Reviewed issue, it’s real, we’d review a pull request. Type:Enhancement A need for something new.

Comments

@Mnkras
Copy link
Contributor

Mnkras commented Jan 19, 2017

Throughout the core, we use FlySystem, and then hardcoded fread/frwrite, file_put_contents, touch etc

These hardcoded paths always assume a local file store which is pretty bad for multiple reasons.

One big thing that stood out for me is that many caches are hardcoded to /application/files/cache, which if you have a distributed site, doesn't work. Each "version" of the site could end up with different out of sync caches.

Currently we also have some things use the default File Storage Location, but most of the time we don't, just those hardcoded paths.

Some stuff that needs to be updated (probably):

  • concrete/blocks/image/tools/composer_save.php
  • concrete/jobs/generate_sitemap.php - Need to look into what to do with this one
  • concrete/src/Marketplace/Marketplace.php
  • concrete/tools/files/importers/remote.php
  • concrete/src/File/Service/File.php
  • concrete/src/Asset/Asset.php
  • concrete/src/Asset/CssAsset.php
  • concrete/src/Asset/JavascriptAsset.php
  • concrete/src/Cache/Page/FilePageCache.php
  • concrete/src/Foundation/Environment.php
  • concrete/src/Package/StartingPointPackage.php - Not sure...
  • concrete/src/StyleCustomizer/Stylesheet.php
  • concrete/src/Updater/ApplicationUpdate.php - Disable In distributed
  • concrete/src/Application/Application.php - setupFilesystem()

This is just a cursory list so far.

@john-everden
Copy link
Contributor

Would love to see this addressed. I will have the need to deploy to a load balanced server later this year when a project launches and this is going to make it difficult.

@Mnkras
Copy link
Contributor Author

Mnkras commented Mar 16, 2017

@aembler Some of the places noted above will always be local to the actual files of the concrete5 installation.

I think the correct way to fix (some) of this is to add a new setting that restricts certain abilities in concrete5.

The new settings would be along the lines of enable_distributed_mode for when concrete5 would run on a distributed system.

When this setting is enabled, certain things that require the local file-system will be disabled. For example:

  • Installing items from the marketplace
  • Saving config entires that save to /application/config (probably need to allow some form of saving...)
  • Updating concrete5 via auto-updater
  • Generate sitemap job

The reason behind that is if you are running the front end on multiple machines these changes are usually version controlled as these physical changes are pushed to all replicas.

What are your thoughts on this?

@aembler
Copy link
Member

aembler commented Mar 17, 2017

It's an interesting idea...there are a lot of places that would be affected...lots of Dashboard pages that would need to reflect this change (for saving config values). It can be an issue for some sites, though.

@Mnkras
Copy link
Contributor Author

Mnkras commented Mar 17, 2017

Yes it could be, but right now, you can get a site into an inconsistent state if you mess with config entries. This isn't something that would be enabled by default (obviously) it would be explicitly be enabled by someone who hopefully has read some documentation.

@aembler aembler added Affects:Developers Affects those who customize the CMS using code. Product Areas:Code Quality Product Areas:Framework Status:Available Reviewed issue, it’s real, we’d review a pull request. Type:Enhancement A need for something new. and removed Recategorize:Discussion labels Jan 8, 2020
@aembler aembler added Enhancement:Level Of Effort:3 Enhancements requiring a significant level of effort. Enhancement:Quality Of Life:1 Enhancements that will have a negligible or small improvement in quality of life. labels Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Affects:Developers Affects those who customize the CMS using code. Enhancement:Level Of Effort:3 Enhancements requiring a significant level of effort. Enhancement:Quality Of Life:1 Enhancements that will have a negligible or small improvement in quality of life. Product Areas:Code Quality Product Areas:Framework Status:Available Reviewed issue, it’s real, we’d review a pull request. Type:Enhancement A need for something new.
Projects
None yet
Development

No branches or pull requests

3 participants