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

Temporary Assets are saved locally and that causes errors when using load balancing #4010

Closed
boboldehampsink opened this issue Mar 19, 2019 · 18 comments
Assignees
Labels
assets 📁 features related to asset management enhancement improvements to existing features

Comments

@boboldehampsink
Copy link
Contributor

boboldehampsink commented Mar 19, 2019

Description

Temporary Assets are saved locally and that causes errors when using load balancing:

yii\base\ErrorException: fopen(/tmp/runtime/assets/tempuploads/user_458/26-kopie.jpg): failed to open stream: No such file or directory in /app/vendor/league/flysystem/src/Adapter/Local.php:181

Why not upload them to a Volume (so we can use S3 for example)?

Steps to reproduce

  1. Create temporary assets by uploading to a field then not saving the entry
  2. Recreate container but not db (mimics load balancing environment)

(I have this issue on Heroku when a "dyno" restarts or is reassigned.

Additional info

  • Craft version: 3.1.18
  • PHP version: 7.3.3
  • Database driver & version: PostgreSQL 10.7
  • Plugins & versions: AWS S3
@brandonkelly
Copy link
Member

Agree, we should probably let you specify a “Temp Uploads Volume” similar to the User Photos Volume.

@brandonkelly brandonkelly added this to the 3.2 milestone Mar 19, 2019
@brandonkelly brandonkelly added enhancement improvements to existing features assets 📁 features related to asset management labels Mar 19, 2019
@ostark
Copy link
Contributor

ostark commented Apr 30, 2019

We had a similar case today (S3 + multiple http nodes). AFAIK Craft stores the assets locally if you define a dynamic Upload Location for the Asset field.

@boboldehampsink @andris-sevcenko can you confirm this?

@rogertinch
Copy link

@brandonkelly this is currently holding up our beta launch on fortrabbit's Pro Stack. Is there any workaround you can suggest for now so we can make our launch deadline?

@brandonkelly
Copy link
Member

Workaround for now would be to mount a file storage volume on all load balanced servers, and point the CRAFT_STORAGE_PATH constant to it in index.php.

define('CRAFT_STORAGE_PATH', '/path/to/mount//storage');

@ostark
Copy link
Contributor

ostark commented Apr 30, 2019

@brandonkelly You know this is not possible on heroku nor fortrabbit

Is using a static Upload Location a workaround? Or is a dynamic path not the reason?

brandonkelly added a commit that referenced this issue May 1, 2019
@brandonkelly
Copy link
Member

Got it. I just implemented a new “Temp Uploads Location” setting for the next Craft 3.2 Alpha release.

Temp Uploads Location setting screenshot

@tinchalamo To get the feature right away, change your craftcms/cms requirement in composer.json to:

"require": {
  "craftcms/cms": "3.2.x-dev#3ef63336e08c30449464dcdd00931dede06cafac as 3.2.0-alpha.4",
  "...": "..."
}

Then run composer update.

Note you will be running the 3.2 Alpha at that point, so you should probably make a database backup first, and make sure you spend a little extra time testing to make sure everything’s still working as expected. (No known breaking changes though.)

@nateiler
Copy link
Contributor

nateiler commented May 1, 2019

Any considerations on simply uploading to the final volume and therefore not needing temporary uploads?

@brandonkelly
Copy link
Member

@nateiler Considered it, but could lead to conflicts with existing folders.

@ostark
Copy link
Contributor

ostark commented May 1, 2019

Wow!
@brandonkelly 👌

@rogertinch
Copy link

@brandonkelly thanks for getting on that so quickly! Any chance you might make this a patch release on 3.1.x so we don't fall out of synch with current 3.1.x updates?

@brandonkelly
Copy link
Member

@tinchalamo Nah it definitely feels like a 3.2 feature. We’re keeping the 3.2 branch in sync with 3.1 though, so you’ll be fine. (We just released 3.2 Alpha 5 earlier today with the new Temp Uploads Location setting, plus everything from yesterday’s 3.1.25 release, for example.)

@rogertinch
Copy link

@brandonkelly looks like this broke CraftQL implementation. Trying this simple query:

query fetchShows {
  entries(section: [shows]) {
		... on ShowsMovie {
      id
    }
  }
}

And this is the error it's giving:

{
  "error": "Trying to get property 'fieldLayoutId' of non-object"
}

@andris-sevcenko
Copy link
Contributor

@tinchalamo there should probably be a stack trace in your log files. Can you share that?

@rogertinch
Copy link

@andris-sevcenko I'm seeing it through the CraftQL UI, what's the best way for me to get a stack trace for that?

@andris-sevcenko
Copy link
Contributor

In the storage/logs folder on the server would be a great place to start :)

@rogertinch
Copy link

@andris-sevcenko can I email it to you since it has a lot info in it I don’t want shared publicly?

@andris-sevcenko
Copy link
Contributor

Of course. Just shoot it over to support@craftcms.com and reference this issue!

@rogertinch
Copy link

Done. Thanks @andris-sevcenko!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assets 📁 features related to asset management enhancement improvements to existing features
Projects
None yet
Development

No branches or pull requests

6 participants