-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
check if local path is valid before writing #4034
Conversation
Can we get this change in a new release?😍 im having the same problem, and this would save my week :).... |
I tagged the release |
Thank you! |
Hi there @adamgoucher , Issue Summary Context Error Details Stack Trace Impact Request My temporary fix was to comment the entire block of code on file src/Writer.php from line 168 to 172 where the fix #4033 was provided. Thank you for your attention to this matter. |
fixes #4033
Please take note of our contributing guidelines: https://docs.laravel-excel.com/3.1/getting-started/contributing.html
Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
1️⃣ Why should it be added? What are the benefits of this change?
When exporting a queued report, if the filesystems are different across members of the cluster doing the exporting, things can error as it writes the temporary file as it was on the first machine.
This addresses #4033
2️⃣ Does it contain multiple, unrelated changes? Please separate the PRs out.
3️⃣ Does it include tests, if possible?
4️⃣ Any drawbacks? Possible breaking changes?
This was manually tested on our infrastructure and appears to work in both
queue configurations.
5️⃣ Mark the following tasks as done:
6️⃣ Thanks for contributing! 🙌