[BUG/QUESTION] Download excel file behind load balancer

bobbybouwmann commented 5 years ago

Prerequisites

[X] Able to reproduce the behaviour outside of your code, the problem is isolated to Laravel Excel.
[X] Checked that your issue isn't already filed.
[X] Checked if no PR was submitted that fixes this problem.

Versions

PHP version: 7.2.18
Laravel version: 5.8.24
Nova version: v2.0.6
Package version: 1.0.2

Description

I've got a kubernetes setup running with Laravel in a container. The containers work fine with Nova and the excel package in general when you have 1 container running. However we have multiple containers running behind a load balancer.

The problem we have with this is that whenever we download the excel sheet or csv using the action from this package we receive a failed download. This is because the download is created in the /tmp directory in one controller and downloaded from a different controller.

The reason for this is the setup of how downloads work. Whenever you perform an action regarding downloading the download url is returned with the path and file name

download: "https://local.site.com/nova-vendor/maatwebsite/laravel-nova-excel/download?path=%2Ftmp%2Flaravel-excel-SzURRyYeVGglGtCWrrnie7NzfdidTJKi&filename=users-1562309499.csv"
name: "users-1562309499.csv"

After the response has been received in Nova the download is being called from the url. In our case this means the load balancer might take a different container and the download fails.

Note: I'm not sure if this problem lays in this package or Nova, but it surely isn't ideal!

So my actual question now! Is it possible to store the file somewhere else before downloading it. So somewhere local S3 or another directory than /tmp. Does this package provide functionality for that?

Steps to Reproduce

Not sure if I can provide this easily, but I can thing along if you need more info

Expected behavior:

The download is returned right away instead of a URL first

Actual behavior:

The download fails because the file doesn't exist in the /tmp directory of the random picked container.

Additional Information

Any additional information, configuration or data that might be necessary to reproduce the issue.

GlennM commented 5 years ago

Hi Bobby,

The multi-server setup should work for the Laravel Excel Nova package as well. Please refer to the Laravel Excel documentation about multi-server setup to see how to set things up.

bobbybouwmann commented 5 years ago

That is perfect! Thanks for sharing that part in the documentation! I have everything up and running now ;)

bobbybouwmann commented 5 years ago

Sorry I had to reopen this issue again! So I thought I had this working locally, but on my acceptance environment it's still failing. The S3 setup is only working whenever you use queues. But in my case I want to use it without queues as well. Is there a possibility for this?

Updating the local_path doesn't work for me at this point, since we have multiple machines and we don't know where the container is running. Sharing a path isn't going to work as well!

I can make a PR for always being able to use S3, but not sure if you want something like this.

patrickbrouwers commented 5 years ago

Perhaps we could store the "download" file on the temporary disk. In your case that would go to S3. The api to the ExcelController then downloads it via the temporary disk instead of just calling the full path that is passed. (Sounds a lot better anyway actually haha :) )

Feel free to PR it! Thanks!

bobbybouwmann commented 5 years ago

I'm not sure how to handle this since the package always sends back the url of the download to the frontend. How would I save the file to own storage and send that url back from the action in Nova?

I will look into making a PR for everyone to use a different local path, queue or not

patrickbrouwers commented 5 years ago

Instead of sending back the url of the download, send back the path and disk where it's stored. Then in the controller call do the actual download based on those 2 instead of the given full path.

bobbybouwmann commented 5 years ago

So that basically means overriding the controllers from the laravel-excel-nova package right? It seems to be the only solution at this point.

I also looked in being always able to store temp files on another disk, but that needs some more work in the core laravel-excel package. Looked simpel at first sight hehe!

GlennM commented 5 years ago

How about using a custom action and using ExportToExcel for storing it to a disk of your choice? See Full control

jbardnz commented 4 years ago

I'm having the same issue on Laravel Vapor as there is no local filesystem. Is there a way to use the remote_disk to temporarily store the file instead of the local disk?

JeffBeltran commented 4 years ago

@jbardnz if you find a solution would you ping me please, same issue

jbardnz commented 4 years ago

@JeffBeltran I did manage to find a workaround that works quite well. I made a custom action with the following code:

class ExportExcel extends ExportToExcel implements WithHeadings
{
    use InteractsWithQueue;

    public function handle(ActionRequest $request, Action $exportable): array
    {

        $resource = $request->resource();

        $filename = '/tmp/' .  $resource::uriKey() . '-' . now()->format('Y-m-d') . '.xlsx';

        $response = Excel::store(
            $exportable,
            $filename,
            $this->getDisk(),
            $this->getWriterType()
        );

        return Action::download(
            Storage::disk($this->getDisk())->url($filename),
            $this->getFilename()
        );
    }

    public function fields()
    {
        return [];
    }
}

This will upload the file to the tmp folder on S3 (so it is removed every 24 hours). Just change the folder location if you don't want it to be deleted. Then on the Nova resource, you can add the action like so:

 (new ExportExcel())->withDisk('my-s3-disk')

JeffBeltran commented 4 years ago

wow thanks man, my brain is dead so i'll save this for the morning. thanks in advance :D

argia-andreas commented 3 years ago

@jbardnz Thank you! Saved me loads of time! Worked like a charm with Vapor. I had to remove the leading / in /tmp/. But might be because of my .env file.

SpartnerNL / Laravel-Nova-Excel