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

Updated docs and added configuration for S3 objectstore upload of media files #2466

Closed
wants to merge 2 commits into from

Conversation

paepke
Copy link

@paepke paepke commented Sep 27, 2018

Docs and configuration examples how to use an S3 backend for uploading media files

Fixes:

#1814

@jeremystretch
Copy link
Member

Any work involving reference to external systems must take special caution to avoid proprietary naming. For instance, we would not introduce a setting named S3_STORAGE_ENABLED as S3 is the brand name of an Amazon product. We want to avoid this because introducing support for additional products would necessitate introducing parallel settings. Instead, settings should use generic names, and any services or libraries requiring specific keys should be transparent to NetBox.

For example, instead of:

AWS_ACCESS_KEY_ID = ''
AWS_SECRET_ACCESS_KEY = ''
AWS_S3_ENDPOINT_URL = ''

We might do something like this:

STORAGE_PARAMETERS = {
    'AWS_ACCESS_KEY_ID': '',
    'AWS_SECRET_ACCESS_KEY': '',
    'AWS_S3_ENDPOINT_URL': '',
}

These arguments can then be passed through to the storage library. This ensures that we can support all current and future settings pertinent to the library without any additional code changes to NetBox.

(I'm not particular suggesting STORAGE_PARAMETERS as a setting name, just illustrating how we can wrap service-specific keys under a generic label.)

@paepke
Copy link
Author

paepke commented Oct 1, 2018

I was thinking over the weekend about your proposal. In the first place it sounds very good, but I'm unsure how to accomplish that in a maintenance friendly way.
The different django-storage backends have very different constants how they are configured like "AWS_", "GS_" or "AZURE_*". Possible plugins to django-storage or future versions will probably follow this naming structure.
There are these solutions with come to my mind:

  1. Copy over the values from the STORAGE_PARAMETERS dict via locals().update(STORAGE_PARAMETERS). This looks kinda messy in my eyes
  2. reassigning the statics by naming them - this would lead to a big block of if/else constructs
  3. writing an additional wrapper for the django-storages plugin. This seems a bit of over the top just for settings the variables in the settings.py correctly
  4. Your ideas?

S3 is not only a product name, its a standard which a lot of other implementations follow and they call themselves "S3 Compliant". Thats why it was ok for me in the first place to use this kind of naming. Additionally the AWS_ prefix is widely used in the documentation of the services which provides a S3 api.
Anyway I highly appreciate your feedback and I'm willing to develop this further to have an good configuration which makes it easy for the user and the codebase to keep this feature maintainable.

@jeremystretch
Copy link
Member

Closing this out for now as it's stalled. Let's continue the discussion in #1814 where it's more visible.

@nward
Copy link

nward commented Apr 22, 2019

@paepke commenting here to alert you that I've discussed my approaches for this in #1814. I think we have similar thoughts on this stuff. I have a PoC of my ideas here https://github.com/SearchLightNZ/netbox/tree/configurable_image_storage - this is a more generic implementation than just "S3" and just django-storages, and is of course compatible with those.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants