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

[dropbox] conform to BaseStorage interface #1251

Merged
merged 1 commit into from
May 20, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 31 additions & 39 deletions storages/backends/dropbox.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,14 @@
# Dropbox storage class for Django pluggable storage system.
# Author: Anthony Monthe <anthony.monthe@gmail.com>
# License: BSD
#
# Usage:
#
# Add below to settings.py:
# DROPBOX_ROOT_PATH = '/dir/'
# DROPBOX_OAUTH2_TOKEN = 'YourOauthToken'
# DROPBOX_APP_KEY = 'YourAppKey'
# DROPBOX_APP_SECRET = 'YourAppSecret``
# DROPBOX_OAUTH2_REFRESH_TOKEN = 'YourOauthRefreshToken'


import warnings
from io import BytesIO
from shutil import copyfileobj
from tempfile import SpooledTemporaryFile

from django.core.exceptions import ImproperlyConfigured
from django.core.files.base import File
from django.core.files.storage import Storage
from django.utils._os import safe_join
from django.utils.deconstruct import deconstructible
from dropbox import Dropbox
Expand All @@ -28,6 +18,7 @@
from dropbox.files import UploadSessionCursor
from dropbox.files import WriteMode

from storages.base import BaseStorage
from storages.utils import get_available_overwrite_name
from storages.utils import setting

Expand Down Expand Up @@ -77,30 +68,15 @@ def _set_file(self, value):


@deconstructible
class DropboxStorage(Storage):
class DropboxStorage(BaseStorage):
"""Dropbox Storage class for Django pluggable storage system."""
location = setting('DROPBOX_ROOT_PATH', '/')
oauth2_access_token = setting('DROPBOX_OAUTH2_TOKEN')
app_key = setting('DROPBOX_APP_KEY')
app_secret = setting('DROPBOX_APP_SECRET')
oauth2_refresh_token = setting('DROPBOX_OAUTH2_REFRESH_TOKEN')
timeout = setting('DROPBOX_TIMEOUT', _DEFAULT_TIMEOUT)
write_mode = setting('DROPBOX_WRITE_MODE', _DEFAULT_MODE)

CHUNK_SIZE = 4 * 1024 * 1024

def __init__(
self,
oauth2_access_token=oauth2_access_token,
root_path=location,
timeout=timeout,
write_mode=write_mode,
app_key=app_key,
app_secret=app_secret,
oauth2_refresh_token=oauth2_refresh_token,
):
if oauth2_access_token is None and not all(
[app_key, app_secret, oauth2_refresh_token]
def __init__(self, oauth2_access_token=None, **settings):
super().__init__(oauth2_access_token=oauth2_access_token, **settings)

if self.oauth2_access_token is None and not all(
[self.app_key, self.app_secret, self.oauth2_refresh_token]
):
raise ImproperlyConfigured(
"You must configure an auth token at"
Expand All @@ -109,16 +85,32 @@ def __init__(
"'setting.DROPBOX_APP_SECRET' "
"and 'setting.DROPBOX_OAUTH2_REFRESH_TOKEN'."
)
self.root_path = root_path
self.write_mode = write_mode
self.client = Dropbox(
oauth2_access_token,
app_key=app_key,
app_secret=app_secret,
oauth2_refresh_token=oauth2_refresh_token,
timeout=timeout,
self.oauth2_access_token,
app_key=self.app_key,
app_secret=self.app_secret,
oauth2_refresh_token=self.oauth2_refresh_token,
timeout=self.timeout,
)

# Backwards compat
if hasattr(self, 'location'):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If location is given in **settings, the super constructor already raises ImproperlyConfigured before this line is reached because location is not in the default settings.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. This is meant to catch the subclassing case where people assign to the property.

That is generally how people have been configuring multiple instances; I’d like to update the code base to accept the OPTIONS from the new 4.2 interface.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. Even before this change, location could not have been passed to the constructor without subclassing, anyway.

warnings.warn(
"Setting `root_path` with name `location` is deprecated and will be removed in a future version of "
"django-storages. Please update the name from `location` to `root_path`", DeprecationWarning)
self.root_path = self.location

def get_default_settings(self):
return {
"root_path": setting('DROPBOX_ROOT_PATH', '/'),
"oauth2_access_token": setting('DROPBOX_OAUTH2_TOKEN'),
"app_key": setting('DROPBOX_APP_KEY'),
"app_secret": setting('DROPBOX_APP_SECRET'),
"oauth2_refresh_token": setting('DROPBOX_OAUTH2_REFRESH_TOKEN'),
"timeout": setting('DROPBOX_TIMEOUT', _DEFAULT_TIMEOUT),
"write_mode": setting('DROPBOX_WRITE_MODE', _DEFAULT_MODE),
}

def _full_path(self, name):
if name == '/':
name = ''
Expand Down