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

Added storage config setting #64

Merged

Conversation

robgaskell
Copy link
Contributor

Added a config item to allow the changing of the ephemeral storage value, used when deploying sidecar-browsershot.

The storage method, which is used by sidecar, returns the value as set in the config. If the config item does not exist, for example if the config has not been updated after this release, the default sidecar value is used.

…lue. Defaults to the sidecar value if the sidecar-browsershot config does not contain the setting
@stefanzweifel
Copy link
Owner

Thanks for the PR.
I don't quite understand, why we need to add this in this package.
If you upgrade wnx/sidecar-browsershot one would also automatically update hammerstone/sidecar.

Or did you run into issues, where this config became a problem?
The config is related to aarondfrancis/sidecar#71, right?

@robgaskell
Copy link
Contributor Author

robgaskell commented Apr 27, 2023

I needed to update the storage for sidecar-browsershot without changing the setting for other sidecar deployed items.

We were getting exceptions such as the following when generating pdfs:

Lambda Execution Exception for Wnx\SidecarBrowsershot\Functions\BrowsershotFunction: "ENOSPC: no space left on device, write. [TRACE] Error: ENOSPC: no space left on device, write at Object.writeSync (fs.js:737:3) at Object.writeFileSync (fs.js:1535:26)".

The PR overrides the method as suggested in the sidecar docs, but using a config item specific to the package, in exactly the same way overriding the default memory config works.

https://hammerstone.dev/sidecar/docs/main/functions/customization#storage

@stefanzweifel
Copy link
Owner

I see, thanks for the explanation.
Yeah having multiple sidecar function and managing the settings for each isn't perfect yet.

Will merge and tag a new version shortly.

@stefanzweifel stefanzweifel merged commit 758908b into stefanzweifel:main Apr 28, 2023
@robgaskell robgaskell deleted the ephemeral-storage-config branch April 28, 2023 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants