<noun>() vs get_<noun>() vs <verb>_<noun>()

@dhermes

Moved over from #910 per @dhermes

Goal of this issue is to decide:

Is it reasonable to provide one-liners to do things like create_topic() which is just a wrapper:
python def create_topic(self, name): topic = self.topic(name) topic.create() return topic
Can we agree on the naming convention of what does and does not require an API request

Comment from the pull request was...

The name create_topic is ambiguous to new users. Does it mean create a Topic instance or actual insert one?

That's a good point. I think we should adopt the attitude that client.<verb>_<noun>() is an API request. So create_topic would hit the API trying to 'insert' the topic (using Google's API lingo). client.<noun> is nothing more than a factory for the noun which won't fire an API request but gives you back an instance of the .

We'd need to offer Client.topic and Client.create_topic side-by-side since users would need to construct pre-existing topics without sending insert.

Totally agree -- I think we had this debate already and I'm totally sold on the need for .topic() as the sole way to get a hold of a topic object, without hitting the API (as the topic literally has zero extra metadata).

For other APIs, where the object itself holds lots of (potentially important) metadata, I think it might be worthwhile to provide a get_<noun>() to pull down that metadata, in addition to the <noun>() method which just gives you an object which may not be tied to the remote (authoritative) representation.

An example here might be bucket.

.bucket() gives me a bucket object, sans remote data (no API request).
create_bucket() explicitly creates the bucket in the remote service, throwing an error if I was unable to do so.
get_bucket() would be an API request that gives me all the metadata about a bucket.

Note that the <verb>_bucket() methods are really just shortcuts so that I can have one-liners for the common things I want to do with a bucket. (ie, create == bucket = client.bucket('name'); bucket.create(); return bucket;)

In boto for S3, they have this verify or check parameter on the bucket constructor, so the comparisons look like:

Action	boto	gcloud (suggestion)
Get a bucket, no API request	`connection.get_bucket(validate=False)`	`client.bucket()`
Get a bucket + metadata	`connection.get_bucket()`	`client.get_bucket()`

I personally don't love the boto method, and would much rather the "gcloud (suggestion)" option.

At a glance, I'll say this is mixing an imperative approach (functions only) with an object oriented approach. By making half the HTTP requests with functions and half with object methods, we muddle what our approach / philosophy is.

That is a good point. My concern is that our staunch adherence to the "only one approach" means more code for people in simple cases.

That is, if we stick to the object-oriented approach, one-liners aren't possible because of our decision to never allow chaining:

# This is *NOT* possible:
topic = client.topic('name').create()

If we go with the imperative approach, one liners are possible, but it means the logic gets mixed up a bit:

topic = client.create_topic()  # What does this return? What can you do with the result?

I'm suggesting we actively decide to make the object-oriented approach the authoritative one, with shortcut helper methods for common simple tasks where a common user would want a one-liner.

class Client(...):
  def create_topic(self, *args, **kwargs):
    topic = self.topic(*args, **kwargs)
    topic.create()
    return topic

This way, all logic lives in one place, but we get the one-liners a lot of users want (me among them)

I'm with most of it but "a lot of users want" isn't clearly true or false (you're the only user we really interact with).

@tseaver

I'm happy to implement, but would love to hear what @tseaver has to say.

I'm -1 on adding more topic- and subscription-specific methods to Client. Trading typing for clarity is not a win to me.

Where is the loss of clarity?

And how do I get a bucket and it's metadata? storage.bucket('...') doesn't do that does it?

from gcloud import storage
b = storage.get_bucket('name')

which is equivalent to

from gcloud import storage
b = storage.Bucket('name')
b.reload()

OK, so we already have the short-cut method for "getting" a bucket.. why not for creating?

from gcloud import storage
b = storage.create_bucket('name')

which would be equivalent to

from gcloud import storage
b = storage.Bucket('name')
b.create()

Right?

@jgeewax

@jgeewax You ought to check out gcloud.storage.api, create_bucket also exists :)

FWIW The reason we have those bucket methods in storage.api is twofold

They were all already implemented on Connection previously
There is no "parent" for a bucket, so they are a top-level concept

So going back to the original thing... topic doesn't have a parent does it? So why wouldn't we add a shortcut method for create_topic too?

@tseaver

The only difference here is that there is no "all already implemented on Connection previously" baggage.

Before, we ported the methods blindly. Now, we can actually think about it.

FWIW, when we ported them @tseaver said he'd prefer we nuke storage.create_bucket and just recommend Bucket.create() and he has continued to mention it for awhile. I mention this to say that it's been clear on our minds, not just an ad hoc stance.

So by that measure we should nuke storage.get_bucket too -- and ask people to do .Bucket() and bucket.reload() -- right?

@tseaver

I don't have strong feelings here, I am just like the club secretary providing the minutes of previous meetings. @tseaver can weigh in

Sure -- my thoughts are that we should decide on something and be consistent. If it's just that the only verb we'll in-line is get, that's fine. I'd also like create and delete to be in-lined but my preference is in that order of priority. (get, then create, then delete). With pubsub, topics don't have any metadata, so get isn't really a big deal, however I think it still should exist...

@dhermes

I think keeping the responsibilities clear is important, and that we should be wary of adding "convenience" methods which confuse them. One symptom of those confused responsibilities is the injection of circular references (such as those @dhermes is fighting in #910 / #912). Another is an explosion of almost-but-not-quite-identical tests for the More Than One Way To Do It implementations.

Re circular references:

Totally get that -- So if that's the case, shouldn't we chop out get_bucket and similar?

Re test explosion:

Should we only have one test for these? Something superficial that says "this should pass along all arguments and call this method... "? And then all the actual test logic is on the real method?

We currently have Client.fetch_project and Client.new_project in Resource Manager, then Client.get_bucket (makes API request) and Client.bucket (factory) in Storage. We don't really have this conflict elsewhere, but should really get our story straight.

Also, these are really the only places that a factory is side-by-side with an API method that does Noun.reload().

fetch_noun() (AFAIK) should just be noun = client.noun() and noun.reload() in one go....

My comment was about how to implement, but about what to implement / what to name the things that are implemented.

We are self-inconsistent right now in our offerings and I'd like to become self-consistent.

This conversation seems to have lost steam on both sides. Closing out, but people can re-open if they think something should be done.

chore(deps): update all dependencies (#911)

chore(python): bump idna from 3.4 to 3.7 in .kokoro (#911)

fix: Allow dropping cleaned-up keys (#911)

chore: group all renovate PRs together (#911)

tests: ensure test runs after reaching retention period (#911)

feat: pass custom Client object to dbapi (#911)

Navigation Menu

Repository navigation

<noun>() vs get_<noun>() vs <verb>_<noun>() #911

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

<noun>() vs get_<noun>() vs <verb>_<noun>() #911

Description

Activity

dhermes commented on Jun 6, 2015

jgeewax commented on Jun 6, 2015

dhermes commented on Jun 6, 2015

dhermes commented on Jun 6, 2015

tseaver commented on Jun 9, 2015

jgeewax commented on Jun 9, 2015

dhermes commented on Jun 9, 2015

jgeewax commented on Jun 9, 2015

dhermes commented on Jun 9, 2015

jgeewax commented on Jun 9, 2015

dhermes commented on Jun 9, 2015

jgeewax commented on Jun 9, 2015

dhermes commented on Jun 9, 2015

jgeewax commented on Jun 9, 2015

tseaver commented on Jun 10, 2015

jgeewax commented on Jun 10, 2015

dhermes commented on Aug 31, 2015

jgeewax commented on Sep 1, 2015

dhermes commented on Sep 1, 2015

dhermes commented on Dec 31, 2015

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions