Changes Unknown when pulling cf58724 on dhermes:fix-566 into * on GoogleCloudPlatform:master*.

Changes Unknown when pulling 5324510 on dhermes:fix-566 into * on GoogleCloudPlatform:master*.

Making an HTTP request at import time is Just Wrong (TM). We should either make an instance of a class with a property which computes __environ__ lazily (maybe __version__ too?), or else ditch the dunder-variable and use a helper function (which could cache the result in a private module global).

I like it. Will re-work with this in mind. I'm a little scared of what unit tests will look like.

Coverage decreased (-0.02%) to 99.98% when pulling 1d4c4de on dhermes:fix-566 into d858ff0 on GoogleCloudPlatform:master.

@tseaver I changed to using a lazy loading property with __str__ defined. I have yet to write tests because I want to discuss (made bogus pragma no cover to avoid).

I also debated using a default USER_AGENT on the class and then on Connection instance creation set a specific user agent? This could be difficult in tests if we create a lot of Connections (I'm not sure if we do).

Why can't GCE be detected without an HTTP request? Bah!

I'm a bit hesitant with this current approach since we have a if self._user_agent is None check before every single HTTP request. This is making me lean back towards setting at import time; the 0.1 second timeout will make it virtually imperceptible.

Another thing I wanted to do: set gcloud-python/0.3.0-regression-test for regression tests. (This can be done without this PR.)

Something like Pyramid's @reify decorator eliminates the lookup overhead.

Maybe we should allow applications to pass in a string to be added to the agent?

1. I like the reify approach, I'll go down that path and ping you again.
2. Allow them to pass a string, that sounds good. I debated just leaving this methods and asking people to set themselves, but realized people don't care about the user-agent. The reason we (or Google) would want the environ set is so we can get more information about how our library is used in the wild.

@tseaver PTAL. I held off on

• allowing a custom user agent
• setting one during regression tests

Will cover in another PR if you're OK with that.

Coverage remained the same at 100.0% when pulling 2f3f5dc on dhermes:fix-566 into d858ff0 on GoogleCloudPlatform:master.

Also I had to add a pragma: NO COVER on the use of the reify decorator. Rather than marked as a partial branch, running coverage html results in it being marked as exit. @tseaver any idea what that means?

Coverage remained the same at 100.0% when pulling b926608 on dhermes:fix-566 into d858ff0 on GoogleCloudPlatform:master.

Deferring the "custom" agent stuff is fine by me.

Or move the logic down into the user_agent method and have the decorator just cache the result of calling in in the instance dict.

We want to leave the logic outside of Connection so that

• branching stops occurring after first call (hence the reify pattern)
• Connection can re-use the environ found without having to make another HTTP request

I'm fine with hard-coding user_agent as the name or with living the pragma: NO COVER.

I was thinking more of something like this gist.

• Is there something inherently wrong with the way I've currently constructed it?
• Also, what is app_id doing in the gist? I'm not sure if a unique user agent per user is a bad thing? (i.e. I don't know what tool Google uses to determine where traffic comes from)
• What happens to Connection.user_agent here? It seems people should be able to just hack it with a string if they like. This ends up requiring something like Connection.user_agent.user_agent

Is there something inherently wrong with the way I've currently constructed it?

Using a decorator to wrap a no-op function smells bad to me.

What is app_iddoing in the gist?

I was addressing #573 (comment). If there were "well known" apps which used the feature, then one might be able to pick their traffic out of the logs.

What happens to Connection.user_agent here?

I had assumed that it would just use the value of gcloud_environment.user_agent (which would be the instance holding the cached / computed value.

• Yes I think wrapping the no-op smells too, I'd rather just hard code the property name as user_agent (it's already a big clue by the name of the class)
• I figured DISCUSSION PR: Add __environ__ to gcloud package to augment user agent #573 (comment) would be more free form
• Assigning Connection.user_agent = gcloud_environment.user_agent at definition time will cause reify to make the HTTP request and we're back in the same situation of an HTTP during import

I figured #573 (comment) would be more free form.

It seemed useful to me to preserve the "library" info along with the application-specific tag.

Assigning Connection.user_agent = gcloud_environment.user_agent at definition time will cause reify to make the HTTP request and we're back in the same situation of an HTTP during import.

I hadn't meant to force it at import time; rather, I would use @reify to wrap Connection.user_agent().

• Where do you envision this class living?
• RE: preserving UA string, I was planning to add a kwarg to Connection and just append (as your gist does)
• Did you mean to have gcloud_environment = GCloudEnvironment() (instead of the class) in your gist?
• Do we actually want the GCloudEnvironment class to be public, or just for demonstration purposes?

@tseaver Rebased on top of #580. Can we resume the conversation?

Coverage remained the same at 100.0% when pulling fd08c42 on dhermes:fix-566 into 9518db7 on GoogleCloudPlatform:master.

Where do you envision this class living?

gcloud/environment.py?

RE: preserving UA string, I was planning to add a kwarg to Connection and just append (as your gist does)

Did you mean to have gcloud_environment = GCloudEnvironment() (instead of the class) in your gist?

Yes: the whole notion requires an instance to hold the cached computed values.

Do we actually want the GCloudEnvironment class to be public, or just for demonstration purposes?

I don't care about the class, per se. I was assuming that the instance might be part of the API, assuming apps might care about using its methods in the same way that the connection does.