Skip to content

Enable anonymous access to blobs in public buckets - #4315

Merged
tseaver merged 2 commits into
masterfrom
4279-storage-create_anonymous_client
Nov 6, 2017
Merged

Enable anonymous access to blobs in public buckets#4315
tseaver merged 2 commits into
masterfrom
4279-storage-create_anonymous_client

Conversation

@tseaver

@tseavertseaver commented Nov 1, 2017•
edited
Loading

Copy link
Copy Markdown
Contributor

/cc @jskeet

Closes#4279

@tseavertseaver added api: storage Issues related to the Cloud Storage API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Nov 1, 2017
@tseaver
tseaver requested a review from dhermesNovember 1, 2017 19:04
@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 1, 2017
dhermes
dhermes reviewed Nov 1, 2017
View reviewed changes

@dhermesdhermes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@lukesneeringer@jskeet Do we expect list_blobs() to be dependable on the gcp-public-data-landsat? (i.e. could we add a system test for that).

@tseaver Don't you need more tests for methods that do and should fail?

Comment threadstorage/setup.py
'google-cloud-core >= 0.28.0, < 0.29dev',
'google-api-core >= 0.1.1, < 0.2.0dev',
'google-auth >= 1.0.0',
'google-auth >= 1.2.0',

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment threadstorage/google/cloud/storage/client.py
:rtype: :class:`google.cloud.storage.client.Client`
:returns: Instance w/ anonymous credentials and no project.
"""
client = cls(project='<none>', credentials=AnonymousCredentials())

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment threadstorage/tests/system.py Outdated
anonymous = storage.Client.create_anonymous_client()
bucket = anonymous.bucket(self.PUBLIC_BUCKET)
for blob in bucket.list_blobs():
print("Downloading blob: {}".format(blob.name))

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment threadstorage/tests/system.py Outdated
print("Downloading blob: {}".format(blob.name))
with tempfile.TemporaryFile() as stream:
blob.download_to_file(stream)
break # just use the first one

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment threadstorage/tests/system.py Outdated
for blob in bucket.list_blobs():
print("Downloading blob: {}".format(blob.name))
with tempfile.TemporaryFile() as stream:
blob.download_to_file(stream)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@tseaver

tseaver commented Nov 1, 2017

Copy link
Copy Markdown
ContributorAuthor

@dhermes

Don't you need more tests for methods that do and should fail?

The back-end enforces the policy by raising 401s: why bother testing them?

@dhermes

dhermes commented Nov 1, 2017

Copy link
Copy Markdown
Contributor

The back-end enforces the policy by raising 401s: why bother testing them?

I sympathize with the sentiment, but am just weary of the cases we haven't thought about.

dhermes
dhermes reviewed Nov 1, 2017
View reviewed changes
Comment threadstorage/tests/system.py Outdated
with tempfile.TemporaryFile() as stream:
blob.download_to_file(stream)
break # just use the first one
blob, = list(bucket.list_blobs(max_results=1))

This comment was marked as spam.

Sign in to view

theacodes
theacodes approved these changes Nov 1, 2017
View reviewed changes

@theacodestheacodes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mostly LGTM

Comment threadstorage/google/cloud/storage/client.py
:rtype: :class:`google.cloud.storage.client.Client`
:returns: Instance w/ anonymous credentials and no project.
"""
client = cls(project='<none>', credentials=AnonymousCredentials())

This comment was marked as spam.

Sign in to view

Comment threadstorage/google/cloud/storage/client.py Outdated

@classmethod
def create_anonymous_client(cls):
"""Factory: return client w/ anonymous credentials.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment threadstorage/tests/unit/test_client.py Outdated
self.assertIsInstance(client._connection, Connection)
self.assertIsInstance(
client._connection.credentials, AnonymousCredentials)
self.assertIsNone(client.current_batch)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@lukesneeringer

lukesneeringer commented Nov 6, 2017

Copy link
Copy Markdown
Contributor

@dhermes Apparently...

ValueError: invalid version number '0.28.1.dev1'
Command exited with code 1

...but only on Windows.

@tseaver

tseaver commented Nov 6, 2017

Copy link
Copy Markdown
ContributorAuthor

@dhermes, @lukesneeringer That spelling is correct according to PEP 440. We could try dev0 (we won't need more than one) or dev (an allowed abbreviation).

@tseaver
tseaverforce-pushed the 4279-storage-create_anonymous_client branch from ed35ccc to 9c685f1CompareNovember 6, 2017 16:09
@tseaver

tseaver commented Nov 6, 2017

Copy link
Copy Markdown
ContributorAuthor

Rebased / semi-squashed to fix Appveyor, tidy up.

@dhermes

dhermes commented Nov 6, 2017

Copy link
Copy Markdown
Contributor

The AppVeyor issue was fixed in #4313 (I just removed bdist_msi from our build since we never used the artifacts). @jonparrott and I discussed, there seems to be a bug in bdist_msi, it uses StrictVersion.

@tseaver

tseaver commented Nov 6, 2017

Copy link
Copy Markdown
ContributorAuthor

@dhermes Yup, that was why I rebased on current master.

@tseaver

tseaver commented Nov 6, 2017

Copy link
Copy Markdown
ContributorAuthor

AFAIK, this PR is ready to merge once CI is green.

@tseaver
tseaver merged commit 1ed56fd into masterNov 6, 2017
@tseaver
tseaver deleted the 4279-storage-create_anonymous_client branch November 6, 2017 18:05
chemelnucfin pushed a commit to chemelnucfin/google-cloud-python that referenced this pull request Nov 6, 2017
@tseaver@chemelnucfin
Enable anonymous access to blobs in public buckets (googleapis#4315)
3377419
chemelnucfin pushed a commit to chemelnucfin/google-cloud-python that referenced this pull request Nov 6, 2017
@tseaver@chemelnucfin
Enable anonymous access to blobs in public buckets (googleapis#4315)
8b5a48b
@tseavertseaver mentioned this pull request Jan 10, 2018
Merged
parthea pushed a commit that referenced this pull request Mar 9, 2026
@tseaver
Enable anonymous access to blobs in public buckets (#4315)
3480918
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lukesneeringerlukesneeringerAwaiting requested review from lukesneeringer
2 more reviewers
@theacodestheacodestheacodes approved these changes
@dhermesdhermesdhermes left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: storageIssues related to the Cloud Storage API.cla: yesThis human has signed the Contributor License Agreement.type: feature request‘Nice-to-have’ improvement, new feature or different behavior or design.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tseaver@dhermes@lukesneeringer@theacodes@googlebot