Skip to content

Conversation

@theacodes
Copy link
Contributor

No description provided.

@theacodes theacodes requested a review from andrewsg May 31, 2017 23:10
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 31, 2017

# Make an authenticated API request
buckets = storage_client.buckets().list(project=project).execute()
print(buckets)
Copy link
Member

Choose a reason for hiding this comment

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

Normally I'd "from future import print_function" in a file that uses print(), even if this technically works in 2.7 as-is. Do we have an explicit policy on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't, and generally because we use .format is isn't ever explicitly needed to use print_function.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, my preference is to include the import because print with parens is confusing syntax in Py2, but I'll leave it to you.

import snippets


def test_implicit():
Copy link
Member

Choose a reason for hiding this comment

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

I am confused about the structure of these tests; is there part of this (that does assertions etc) I'm missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a system test, in this case an absence of an error is the assertion. We could technically capture the output and assert that it matches some idea of what we think the output should be, but it doesn't necessarily add any value.


def implicit():
import google.auth
from google.auth.transport import requests
Copy link
Member

Choose a reason for hiding this comment

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

Why do we do these imports inside the function? And why do we have a vendored requests version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because when these are included in the documentation it will only include the body of this function.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, this strikes me as a bit confusing when you're reading the actual file, but not a big deal.

@theacodes theacodes merged commit 8bb93f8 into master Jun 1, 2017
@theacodes theacodes deleted the auth-samples branch June 1, 2017 19:00
gguuss pushed a commit that referenced this pull request Jun 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants