Changes between Initial Version and Version 1 of Ticket #27590, comment 13


Ignore:
Timestamp:
Nov 23, 2020, 2:11:13 AM (4 years ago)
Author:
David Sanders

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #27590, comment 13

    initial v1  
    11Kevin, I think you're neglecting how `collectstatic` is used, and has been historically used. Changing the default to storing the manifest in the codebase would be a breaking change for some users. The `ManifestFilesMixin` hashes files based on content, and the content is going to depend on people's configurations, and the content will likely differ depending on the settings used between development and production. Minifying JS for production is a common use-case, but not for development. If the manifest by default lives in the codebase, it would both need to be re-generated after any changes (change in development process), and would end up with the output for the development environment. Django would probably need to give a large caveat to ignore the file for source control otherwise users will shoot themselves in the foot quite often.
    22
    3 The issue I originally brought up with `S3BotoStorage` is that `ManifestFilesMixin` is a mixin, so adding that to a storage class like `S3BotoStorage` means it will read and write the manifest from that storage - in that case from S3. Changing the behavior so that the manifest doesn't use that storage class could be unintuitive for users. It will have particular ramifications to how and when they call `collectstatic` for their deployments - when using a class like `S3BotoStorage` that sends the files to remote storage, they may not be currently invoking `collectstatic` at a point where it can write out the manifest into the codebase and have that change be persistent and make it into the deployment of the code to production as well. So there'd be a lot of assumptions about how people are currently using it.
     3The issue I originally brought up with `S3BotoStorage` is that `ManifestFilesMixin` is a mixin, so adding that to a storage class like `S3BotoStorage` means it will read and write the manifest from that storage - in that case from S3. Changing the behavior so that the manifest doesn't use that storage class could be unintuitive for users. It will have particular ramifications to how and when they call `collectstatic` for their deployments - when using a class like `S3BotoStorage` that sends the files to remote storage, they may not be currently invoking `collectstatic` at a point where it can write out the manifest into the codebase and have that change be persistent and make it into the deployment of the code to production as well. For example, if a Docker image is used to deploy to multiple instances, currently someone with that setup would probably run `collectstatic` once from a single instance during a deploy - and at that point the manifest would have to be re-baked into the image or it wouldn't exist on other instances, etc. For example, AWS Elastic Beanstalk guides show configuring such that `migrate` is run at deploy time in the containers, and they use `leader_only` so only one container runs it. Someone using S3 for storing the staticfiles might logically have a setup on Elastic Beanstalk where they'd use `leader_only` with `collectstatic` so that N containers don't try to send the files off to S3. So there'd be a lot of assumptions about how people are currently using it.
    44
    55I haven't looked at this issue in 4 years, so that's going from memory. But I don't believe there's a simple "change the default location" solution given how many ways `collectstatic` might be used - you're probably going to break it for somebody. If there were a clear win here I think I probably would have just made a PR instead of this issue. :-) I spent a decent amount of time thinking through the problem before punting it to this issue. I fear if you make it a hard-coded change to the default, even with a deprecation cycle you'll end up with people coming out of the woodwork on why they *have* to have it working the way it originally did, and if there's no configuration ability then you've got a problem there.
Back to Top