- 🇨🇦Canada deviantintegral
We asked our contacts at Pantheon about the stability of these APIs and they said:
Today, the formal answer is that that API is internal and you shouldn't rely on it, but in practice we're unlikely to change it without lots of advance notice due to aforementioned leakiness. The intention is to eventually expose these features via the PantheonAPI, and provide formal wrapper libraries for use on the platform
So far, we've had no issues downloading large PDFs. If that was a problem, I would expect intranet sites on Pantheon would be totally broken since all of those files would likely be in the private file system.
For now, we're going to leave our current implementation as is, but if we have troubles with larger files we'll come back to this.
- 🇨🇭Switzerland berdir Switzerland
Unclear about the status here, don't forget set an according issue status, I rarely look at active feature requests as I assume there's nothing to review.
So this only adds an event subscriber in the MR, I'd be open to supporting this, do have some thoughts:
* it seems to overlap with ✨ Introduce hook to alter the download response Active , we probably don't want a hook *and* an event. lets make sure that the event subscriber is flexible enough to cover that other use case?
* that seems like a lot of code, I would expect that stage_file_proxy would offer an api to do this, no? - Status changed to Needs review
over 1 year ago 10:55pm 26 April 2023 - last update
over 1 year ago 3 pass - 🇨🇦Canada deviantintegral
it seems to overlap with #3351543: Introduce hook to alter the download response, we probably don't want a hook *and* an event. lets make sure that the event subscriber is flexible enough to cover that other use case?
I initially agreed, but then looking at the other issue I think they should probably just decorate the entire controller.
I'm not sure what additional events make sense to fire. Like if you want to alter the file URI, you probably want to do that globally in an existing entity hook or event. The stream wrapper manager can be swapped with injection as well. And we already call hook_file_download. Anyone else have insight here?
that seems like a lot of code
No, there's no real API for this in Stage File Proxy. Nearly all of that code is copied or influenced by
\Drupal\stage_file_proxy\DownloadManager
which is currently final and internal. There could be in the future - it's more that when we did the refactor to add that class, we weren't sure what a good public API surface would be so we kept it restricted to start.Let's go to review, so at the least we can get some next steps on this.
- 🇪🇨Ecuador LeonelEnriquez98
I rerolled the patch to be compatible with version
8.x-2.3
- Status changed to RTBC
2 months ago 1:00pm 10 October 2024 - 🇨🇭Switzerland berdir Switzerland
Update merge requests, not patches. Patches can't be tested and will not be committed. It's generally recommended to just store patches from merge requests locally now.