Implementing resource manager API

[dhermes]

FYI @jgeewax this is "reclaimed" from #933.

@tseaver We can either review as-is or I can break off each commit as a PR.

I'm unclear how we can run system tests on Travis since they require a user account. We could always create a fake Google user account, but that may be frowned upon. For the oauth2client system tests, there is just a refresh token for my personal GMail account (only scope on there is email).

[dhermes]

@jgeewax I should note that I made some changes from your original

• Got rid of Client.get_project (per discussion in <noun>() vs get_<noun>() vs <verb>_<noun>() #911). However I made it very clear in the docstrings of both Client.project (factory) and Project.reload that they needed one another to actually get data.
• Added all Project constructor args to the Client.project factory
• Used Py3 print(project) in docstrings
• Trimmed down many docstrings (i.e. now just one-liner docstrings on each module, most methods no longer have example snippets)
• Adding august 2015 next to the parenthetical currently
• Made the constructors fail if service account credentials were used (also disabled the from_service_account_* factories for the Client)
• Moved _ProjectIterator definition to module-level in Client instead of defined in the method call.
• Switched the order of client and name in the Project constructor
• Made _require_client just emulate other uses (not throw a ValueError) since a missing client will result in (None).connection eventually, which is an AttributeError (LBYL vs. EAFP)
• Made the reload_data flags default to False instead of True (explained in f79c049)
• Making the dummy test _Client a container for connection and nothing else. (It previously inherited from the real Client, we avoid leakage between modules for tests at all costs)

[jgeewax]

now just one-liner docstrings on each module, most methods no longer have example snippets

I'm hugely against this one -- the example snippets next to the code was a big debate for us (IIRC) but we ran a UX study for gcloud-node and those examples were a major factor for usability.

Made _require_client just emulate other uses (not throw a ValueError) since a missing client will result in (None).connection eventually, which is an AttributeError (LBYL vs. EAFP)

Not familiar with this - can you explain a bit more...? Why wouldn't we have an error happen immediately?

[jgeewax]

Got rid of Client.get_project (per discussion in #911). However I made it very clear in the docstrings of both Client.project (factory) and Project.reload that they needed one another to actually get data.

OK so ... to "load a project from the API" I need to do:

from gcloud import resource_manager
client = resource_manager.Client()
project = client.project('my-project')
project.reload()

?

That seems crazy to me. Why wouldn't we provide a get_ or load_ project ? From my perspective, it's sacrificing usability...

[dhermes]

In your UX study, were participants jumping into the source code? How / where did they see these examples? I don't think many "average" Python users even know how to find the source after they have pip installed a library.

EAFP: https://docs.python.org/2/glossary.html#term-eafp
LBYL: https://docs.python.org/2/glossary.html#term-lbyl

---

As an aside RE: examples in docstrings, I know

• @tseaver has strong-ish feelings on this
• it contradicts our current style

Rather than have that block / relate to this PR, we should have a new discussion (in an issue) and we can revisit these docstrings (and others) after that discussion is resolved.

---

As for the loading of a project, I'm happy to have two methods, but not called project and get_project. Having to explain why our names are confusing / subtle just means we need new names.

Also calling it "crazy" is a bit much. We have a clear style in this library of using an object oriented approach, contradicted in only one place get_bucket, which we had discussed removing in #911.

[jgeewax]

In node, they weren't looking at code -- they were looking at the method
listing, and were far more productive if there was an example next to the
method (ie,
https://googlecloudplatform.github.io/gcloud-node/#/docs/v0.20.0/datastore/dataset?method=runInTransaction
)

If we have that, I'm happy.

Deleting and losing those examples is sad. Can we keep them somewhere to
re-open the discussion at the very least?

---

I'm happy to have two methods, but not called project and get_project.
Having to explain why our names are confusing / subtle just means we need
new names.

OK, so what do you want to call it?

Also calling it "crazy" is a bit much.

Providing no single method to load data from an API is (IMO) "crazy". Our
primary goal has always been usability. Consistency is a big ingredient
in that, but I think we are making the wrong choice here by saying "Here's
how you load a project: run this method to get an object, then call reload
on it. Because we said so."

[dhermes]

Deleting and losing those examples is sad.

They'll remain around until you delete your branch. (I also have your branch checked out locally on 2 or 3 machines.)

---

just means we need new names.

I'm all ears.

[jgeewax]

load_project? fetch_project?

[dhermes]

👍 to fetch_project

Thoughts on a name for project, maybe project_factory?

---

Or what about this instead:

project1 = client.project(project_id1, fetch=True)
project1.name = 'New name'
project1.update()

project2 = client.project(project_id2, name='Foo',
                          labels={'bar': 'baz'}, fetch=False)
project2.create()

UPDATE: I'm unclear on which should be the default, but we could always make fetch required.

[jgeewax]

fetch_project

I'm a fan of fetch_project, but not a fan of project_factory.

Looking at the two use cases:

1. Load a remote project
2. Create a new project

project = client.fetch_project('project-id')  # SGTM
project = client.project_factory('project-id')  # Ugly :(
project.create()  # SGTM

---

The parameter seems to hide this as a "not so important" thing but it actually changes the meaning of the thing... How about fetch_project and new_project (boto uses 'new'):

project = client.fetch_project('project-id')
project = client.new_project('project-id')
project.create()

?

[dhermes]

I think I can work with new_ but think the fetch parameter may be cleaner.

As for hiding the "not so important" thing I agree with you. We could do something like

    def project(self, project_id, ..., fetch=None):
         if fetch is None:
            raise ValueError('The fetch argument must be True or False.')
         ...

to ensure that the importance was clear.

[jgeewax]

Oh jeez. Required parameter like that is even scarier for me. Much happier with new and fetch.

[tseaver]

FWIW, I'm a big fan of examples in narrative docs (which is where we want new users to start). I don't see the win for them inside reference docs (which are for refreshing one's memory of how the API works, what parameters it requires/ allows).

Another effect of doctest examples inside already-long docstrings: they can make the signature of a function/method can be buffers above the actual body, which makes reading the code much harder.

[jgeewax]

I don't see the win for them inside reference docs (which are for refreshing one's memory of how the API works, what parameters it requires/ allows).

The study pointed partly to this (people do want to see what parameters are required) but wanted to see examples of using the method as well. Based on that feedback, I see a big win.

they can make the signature of a function/method can be buffers above the actual body, which makes reading the code much harder.

I think relatively few people will read the actual code, far more will read the docs that get generated and live in the gh-pages branch.

---

To be completely clear, I don't see the value in documenting things like from_api_repr() with examples (the only people ever using that should be people editing code of gcloud-python). For methods like list_projects() I think an example is extremely useful and helps people writing their own code ("Hmm list projects, that sounds right... what's the recommended way to do this...?"), and reading others' code ("Hmm.. this code is using list projects, I wonder if they're doing it 'the right way'?")

[dhermes]

Can we move this philosophical discussion elsewhere? It doesn't need to block this PR: we can always add or remove comments and documentation as we like once the code is committed.

[jgeewax]

Did we agree on fetch + new?

[dhermes]

We did not explicitly agree, but I'm happy to carry it out.

---

FWIW, I wouldn't call my suggestion scary but it's not idiomatic. I should have just suggested the Pythonic thing, making a param required by making it positional:

    def project(self, project_id, fetch, name=None, ...):
         ...

and then we could just have all our examples refer to it by name

existing_project = client.project(project_id, fetch=True)

brand_new_project = client.project(project_id, name='New Thing', 
                                   labels={...}, fetch=False)

[jgeewax]

It feels wrong to me to have a parameter affect the "action" happening

client.project(project_id, fetch=True) # means "load from a remote resource"
client.project(project_id, fetch=False) # means "factory: give me an instance"

I'm very much +1 to having two separate methods for the two separate actions -- we just need to decide what to call them....

[dhermes]

@jgeewax Added it as the final commit in this PR. PTAL.

[jgeewax]

Mostly good to me, going to create two issues:

1. Add examples back for all methods people would actually use
2. Switch docs to use semantic line-breaks ("for the most part, just hit enter when you would pause if reading aloud")