Allow custom endpoints with Google Cloud Storage. #648
Conversation
martey
force-pushed
the
gcs-custom-endpoint
branch
2 times, most recently
from
5b42349
to
cb0c0c4
Compare
martey
force-pushed
the
gcs-custom-endpoint
branch
from
cb0c0c4
to
17115bd
Compare
|
I've updated this commit so that it is compatible with google-cloud-storage 1.15.0 (the addition of support for v4 signed URLs broke one of the imports added here). Because v4 signed URLs are still in beta, this commit still uses v2 signed URLs. I've updated the google-cloud-storage minimum version to 1.15.0, but I can change it to be backwards compatible if necessary. |
martey
force-pushed
the
gcs-custom-endpoint
branch
from
17115bd
to
6923be4
Compare
|
Hi @jschneier, can you let me know if there is anything else I should do to help this get merged? |
|
+1 would like this as well @jschneier |
martey
force-pushed
the
gcs-custom-endpoint
branch
from
6923be4
to
9c8f910
Compare
|
Hi, @sww314 and @jschneier, we are using this PR on production for a couple of months, and it works well. Can you give some advices to help this get merged? Thanks. |
martey
force-pushed
the
gcs-custom-endpoint
branch
from
9c8f910
to
a2f8898
Compare
|
I've rebased this so that it can merge cleanly with |
|
I'm also interested in this feature! |
martey
force-pushed
the
gcs-custom-endpoint
branch
from
a2f8898
to
e1c26bd
Compare
|
I've updated this to fix a bug with generating signed URLs and to stop using |
| @@ -19,7 +19,7 @@ def read(filename): | |||
| 'boto': ['boto>=2.32.0'], | |||
| 'boto3': ['boto3>=1.4.4'], | |||
| 'dropbox': ['dropbox>=7.2.1'], | |||
| 'google': ['google-cloud-storage>=0.22.0'], | |||
| 'google': ['google-cloud-storage>=1.15.0'], | |||
There was a problem hiding this comment.
The code in this pull request relies on the fact that Blob.generate_signed_url has a api_access_endpoint parameter (introduced in googleapis/google-cloud-python#7460 and google-cloud-storage 1.15.0).
It would be theoretically possible to update the code so that it is still compatible with google-cloud-storage 0.22.0, but because of the substantial differences in google-cloud-storage pre- and post-1.15.0, this would take a substantial amount of work and would significantly increase the complexity of this pull request's code.
There was a problem hiding this comment.
That change was merged in April. I'm not sure if I can unilaterally require that upgrade.
Is the major difference that non-public URLs wouldn't be able to be signed?
|
I see there's quite a bit of demand for this, what did a previous iteration look like that didn't mandate the version upgrade? I want to merge some variant of this. |
The "previous iteration" from January of this year (cb0c0c4) was created before google-cloud-storage 0.15.0 existed, and isn't compatible with any releases >= 0.15.0. It wouldn't be a valid option unless we wanted to require users have a version of google-cloud-storage before 0.15.0 or (as you mentioned in your earlier comment) were willing to prevent non-public URLs from working. I am unsure about whether backporting these changes to google-cloud-storage < 0.15.0 would be successful. The version requirement on google-cloud-storage in setup.py encourages installing the most recent version of google-cloud-storage, so the automated tests don't cover using an older version of google-cloud-storage and it's possible that there are additional hidden bugs or problems in those older versions. It's also not clear that older versions like 0.22.0 are supported by Google (google-cloud-storage's changelog starts with version 1.6.0). Personally, I don't think mandating a library upgrade is a huge ask for a new feature, especially when it would only affect people who have explicitly pinned google-cloud-storage. That said, I am still interested in having this merged, and will try to make whatever changes are necessary to have this happen. |
|
You make good points...especially that our version checking is loose see e.g the failing tests on the PR. That one would have been caught by running the tests every day though. At some point I will need to rip the bandaid off and remove all of the Python 2 related and deprecated things. Since I have historically been willing to break backwards compat when I increment minor versions I don't think it's too much ask to require it for the next one but it does leave open a question of when that should be done. Can you please rebase to fix the tests. |
martey
force-pushed
the
gcs-custom-endpoint
branch
3 times, most recently
from
ad3bab9
to
8aeda61
Compare
Add new `GS_CUSTOM_ENDPOINT` setting to allow usage of custom domains/URLs with Google Cloud Storage, similar to `AWS_S3_CUSTOM_DOMAIN`. This upgrades the minimum version of google-cloud-storage to 1.15.0.
martey
force-pushed
the
gcs-custom-endpoint
branch
from
8aeda61
to
334be00
Compare
|
I've rebased this and ensured that all tests pass. |
Add new
GS_CUSTOM_ENDPOINTsetting to allow usage of custom domains/URLs with Google Cloud Storage, similar toAWS_S3_CUSTOM_DOMAIN.