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

Keep backups, even when they're old. #2734

Merged
merged 1 commit into from Sep 22, 2020
Merged

Keep backups, even when they're old. #2734

merged 1 commit into from Sep 22, 2020

Conversation

braddunbar
Copy link
Contributor

Recently, we encountered an issue with webpacker removing our previous assets during the webpacker:clean task, and not keeping the specified number of backups. We expected the behavior from sprockets, in which assets are kept if under age or within the count limit. However, webpacker only keeps assets if under age AND within the count limit.

I think this should work like sprockets and has perhaps been overlooked.

Recently, we encountered an issue with webpacker removing our previous assets
during the webpacker:clean task, and not keeping the specified number of
backups. We expected the [behavior from sprockets][sprockets], in which assets are
kept if under age or within the count limit. However, webpacker only keeps
assets if under age *AND* within the count limit.

I think this should work like sprockets and has perhaps been overlooked.

[sprockets]: https://github.com/rails/sprockets/blob/358f83ff09a77f69ac17543a9b1d127737060f00/lib/sprockets/manifest.rb#L258-L259
@gauravtiwari gauravtiwari merged commit 94f44c2 into rails:master Sep 22, 2020
@gauravtiwari
Copy link
Member

thank you 🍰

@braddunbar braddunbar deleted the clean-task branch September 22, 2020 09:28
braddunbar added a commit to harvesthq/webpacker that referenced this pull request Sep 22, 2020
Recently, we encountered an issue with webpacker removing our previous assets
during the webpacker:clean task, and not keeping the specified number of
backups. We expected the [behavior from sprockets][sprockets], in which assets are
kept if under age or within the count limit. However, webpacker only keeps
assets if under age *AND* within the count limit.

I think this should work like sprockets and has perhaps been overlooked.

[sprockets]: https://github.com/rails/sprockets/blob/358f83ff09a77f69ac17543a9b1d127737060f00/lib/sprockets/manifest.rb#L258-L259
braddunbar added a commit to harvesthq/webpacker that referenced this pull request Sep 22, 2020
Recently, we encountered an issue with webpacker removing our previous assets
during the webpacker:clean task, and not keeping the specified number of
backups. We expected the [behavior from sprockets][sprockets], in which assets are
kept if under age or within the count limit. However, webpacker only keeps
assets if under age *AND* within the count limit.

I think this should work like sprockets and has perhaps been overlooked.

[sprockets]: https://github.com/rails/sprockets/blob/358f83ff09a77f69ac17543a9b1d127737060f00/lib/sprockets/manifest.rb#L258-L259
@Pezmc
Copy link

Pezmc commented Sep 28, 2020

@gauravtiwari Can you point me to any docs on releasing webpacker updates? I'd love to help backport this bugfix to 5.X.X, as we're currently using a custom fork to get this fix, and cut a release!

KSJake added a commit to KennaSecurity/webpacker that referenced this pull request Sep 28, 2020
@dkniffin
Copy link

dkniffin commented Feb 3, 2021

@gauravtiwari @Pezmc +1. We just encountered this issue as well and it caused lots of missing assets on our prod env. I'd really like to see it back-ported to 5.X.X as well.

@oboxodo
Copy link

oboxodo commented Feb 3, 2021

+1 to backporting this to 5.x! we were bitten by this issue in production today!

oboxodo pushed a commit to Quimbee/webpacker that referenced this pull request Feb 3, 2021
Recently, we encountered an issue with webpacker removing our previous assets
during the webpacker:clean task, and not keeping the specified number of
backups. We expected the [behavior from sprockets][sprockets], in which assets are
kept if under age or within the count limit. However, webpacker only keeps
assets if under age *AND* within the count limit.

I think this should work like sprockets and has perhaps been overlooked.

[sprockets]: https://github.com/rails/sprockets/blob/358f83ff09a77f69ac17543a9b1d127737060f00/lib/sprockets/manifest.rb#L258-L259
@Pezmc Pezmc mentioned this pull request Feb 4, 2021
4 tasks
@Pezmc
Copy link

Pezmc commented Feb 4, 2021

#2912

@dkniffin
Copy link

dkniffin commented Feb 5, 2021

It turns out our issue was a bit different than this. This issue certainly didn't help us, but wasn't the root cause of our issues. I've created a GH issue over at #2913 describing what we ran into.

gauravtiwari pushed a commit that referenced this pull request Apr 27, 2021
Recently, we encountered an issue with webpacker removing our previous assets
during the webpacker:clean task, and not keeping the specified number of
backups. We expected the [behavior from sprockets][sprockets], in which assets are
kept if under age or within the count limit. However, webpacker only keeps
assets if under age *AND* within the count limit.

I think this should work like sprockets and has perhaps been overlooked.

[sprockets]: https://github.com/rails/sprockets/blob/358f83ff09a77f69ac17543a9b1d127737060f00/lib/sprockets/manifest.rb#L258-L259

Backport-PR-URL: #2912

Co-authored-by: Brad Dunbar <dunbarb2@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants