set_default_project living inside each service module is confusing

Looks like inside each service module (let's say datastore and pubsub) we import _helpers.set_default_project.

If that's the case, I'd expect the following code snippet to have different projects set for the different services:

from gcloud import datastore, pubsub
datastore.set_default_project('datastore-project')
pubsub.set_default_project('pubsub-project')

assert 'datastore-project' == datastore.get_default_project()
assert 'pubsub-project' == pubsub.get_default_project()

If that's not the case, we need to figure out some other way of doing this... maybe moving the helper method into the top level as gcloud.set_default_project()...

@jgeewax

@jgeewax It seems you're just concerned with the fact that the method lives in each sub-package?

We discussed the idea of a library-wide default project with you at a standup meeting and you signed off on it. (@tseaver noted this immediately after.)

The method is only defined in one place gcloud._helpers but it is imported as a public method (i.e. no import ... as rename) in both storage.__init__ and pubsub.__init__.

This is so that we can call it from set_defaults() in each of those packages.

An easy (partial) way to address your concern will just be

from gcloud._helpers import set_default_project as _set_default_project

A remaining issue will be "confusion" if someone does something explicit / declarative like

from gcloud import pubsub
from gcloud import storage
...
pubsub.set_defaults(connection=conn2, project='proj2')
storage.set_defaults(connection=conn1, project='proj1')

because the value 'proj1' will become the global (not just pubsub only) default. A "wrong global" would not be that big an issue for storage since the project is seldom used. It would be a huge issue for pubsub since a project is sent in every request.
3. We could address the above point by either removing project= from set_defaults or by making the default project local to each sub-package.

You're correct in the case of (2) -- we expose the method under each service, but it's not specific to the service. That's pretty confusing to me -- I'd expect a separate default for the service in which I called the method...

Option 4 might be to have a gcloud.get_default_project() and then service specific ones that override the global one... But that's getting pretty tricky...

Yeah I had thought of Option 4. It would be a lot of code for a non-crucial feature.

It seems you're just concerned with the fact that the method lives in each sub-package?

This is correct.

This brings about a further issue that I'm curious about:

from gcloud import pubsub
from gcloud import storage
...
pubsub.set_defaults(connection=conn2, project='proj2')
storage.set_defaults(connection=conn1, project='proj1')

What is the default connection used when talking to Pub Sub here? I think it's conn1 (even though I set the "pubsub connection" to be conn2)...

So:

Why do we need set_defaults as separate methods inside each module?
If we don't have a great reason, are there any objections to just... pulling it out?

If we had the defaults pulled out:

import gcloud
gcloud.set_defaults(connection=conn, project='project')
# No way to set any 'module-specific' defaults.

Assigning back to you to take a look :)

We don't have any cross-package objects / data other than the project string. The connections in each sub-package are only able to talk to their own service.

Yep, so pubsub.set_defaults() does the exact same thing as storage.set_defaults()... Is there a reason for that? Legacy?

No. They both set a global project (which I am uneasy with and is the point of this) but the connection is only package specific.

The interface is the same but the internals are different.

OK so the connection is package specific... Totally misread.

So, Option 3 above has two pieces:

remove the project= from the defaults
make project specific to a connection

Do you have a preference?

@tseaver

"make project specific to a connection" makes sense for pubsub but not for storage. In storage only 2 out of 34 API requests use the project.

I was a fan of having the default project by package specific, but @tseaver was more enthusiastic about having it be a global than I was about having it be package specific. I can be persuaded either way.