Add Google Cloud Storage backend using the gcloud-python library #236
Conversation
Codecov Report
@@ Coverage Diff @@
## master #236 +/- ##
==========================================
+ Coverage 72.87% 75.32% +2.44%
==========================================
Files 10 11 +1
Lines 1379 1552 +173
==========================================
+ Hits 1005 1169 +164
- Misses 374 383 +9
Continue to review full report at Codecov.
|
storages/backends/gcloud.py
Outdated
| self._file = None | ||
|
|
||
|
|
||
| class GoogleCloudStorage(Storage): |
There was a problem hiding this comment.
@eirsyl The existing backends all use the @deconstructible decorator here but I'm not sure why. Commit comments suggest it has something to do with migrations but I'm able to migrate to a FileField using this backend for storage with this code as is. Therefore I'm inclined to leave the decorator out and if someone needs it, they can add it. What do you think?
There was a problem hiding this comment.
Never mind, I figured out why the decorator is needed and added it.
|
I reviewed the code and made some minor changes. It looks good - very close to the S3 implementation, but S3 and Google Cloud Storage are very close to begin with. Onto tests! |
|
@eirsyl @jschneier Does this look reasonable? Next steps on our end are that we'll be using this in our staging environment within the next 2 weeks. |
|
Great work! Looks good to me 👍 |
|
@jschneier We've been using this successfully in our staging environment for several days. Would you be able to merge it soon please, or let me know what's still missing? |
|
@jschneier Can you please take a look at this? The README says to bug you until it's merged, so that's what I'm doing 😸. It also said to add myself to AUTHORS so I added myself and @eirsyl. @eirsyl please let me know if you want a different name or don't want to be credited. |
|
@jschneier Will you have time to look at this sometime soon please? @ZuluPro Is this something you could look at instead? (I can't tell if you have write access but it looks like you might.) |
|
Yes, currently on vacation but have set aside Monday afternoon to do maintenance on this library, thanks for your patience.
… On Feb 10, 2017, at 14:36, Jody McIntyre ***@***.***> wrote:
@jschneier Will you have time to look at this sometime soon please?
@ZuluPro Is this something you could look at instead? (I can't tell if you have write access but it looks like you might.)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
I think there might be a renewed interest in getting this PR merged given the events that unfolded today :/ |
|
@jschneier Just a reminder: can you please review this soon? I've updated AUTHORS again to fix the merge conflict. |
|
This is great development, and I hope it gets merged soon. I'd like to present one wish though - please take #281 into account and make use of tempfile.SpooledTemporaryFile optional / configurable. |
|
@jschneier Friendly ping. |
|
Would also love this to be merged, as the current solution for GCS is far from ideal ... @scjody It crashes on these imports
However, I can do these 3 imports just fine in a python shell. |
|
Hi. Can someone articulate what the issue is with the current When we do merge this I'd like to deprecate |
|
@jschneier I believe that this pull request depends on |
|
As extra note on the explanation above:
|
|
@jschneier It's mostly about authentication as others have articulated above. (I've personally tested the new functionality using an instance's service account and with a developer's user account, but I expect any authentication type supported by the I also believe that since the new backend uses the official Google library directly and does not require a compatibility mode intended only for short term data migrations, it is more likely to be reliable in the long term. |
|
@mhindery Are you sure you're in the same virtualenv in both cases? Can you try debugging the |
The "gcloud" Python module is now called "google.cloud", so use the new name. This means the name of this module needs to change too, since it can no longer be "google".
This decorator is required to allow Django 1.7 migrations to serialize
instances of this class. Without the decorator, running "makemigrations"
on code like the following will fail with a "Cannot Serialize" error.
from storages.backends.gcloud import GoogleCloudStorage
gcs = GoogleCloudStorage()
class SomeModel(models.Models):
some_field = models.FileField(storage=gcs)
According to https://googlecloudplatform.github.io/google-cloud-python/stable/storage-blobs.html#google.cloud.storage.blob.Blob.upload_from_file it's not actually necessary to specify a MIME type.
We want to use this in the Google Cloud Storage backend as well. This is a separate commit so we can verify the clean_name tests in s3boto still pass (before I move those as well).
This is used by s3boto to support the AWS_LOCATION setting, which isn't needed by Google Cloud Storage
Based on #146
This uses the native
gcloud-pythonlibrary, which is recommended by Google for production setups and allows finer-grained authentication than current solutions (which only work when Google Cloud Storage is used in S3 compatibility mode).