Skip to content

dataset.get's example code looks... broken? #901

Description

@jgeewax

Looking at http://googlecloudplatform.github.io/gcloud-node/#/docs/v0.23.0/datastore/dataset?method=get

I see

Where you see transaction, assume this is the context that's relevant to your use, whether that be a Dataset or Transaction object.

(empty area that looks like it should have code)

Get a single entity.

var key = dataset.key(['Company', 123]);

transaction.get(key, function(err, entity) {});

I think there are a few issues

  1. The empty space looks like it should have code
  2. The second snippet about getting a single entity refers to a variable that doesn't appear to be defined

Any idea what's up here?

Activity

  1. callmehiphop commented on Sep 30, 2015

    @callmehiphop
    Contributor

    The empty space looks like it should have code

    Looks like this is just a formatting error, at some point Get a single entity. was actually written in as a JS comment, it looks like when it was changed it created the empty code block.

    http://googlecloudplatform.github.io/gcloud-node/#/docs/v0.14.0/datastore/dataset

    The second snippet about getting a single entity refers to a variable that doesn't appear to be defined

    I believe the intro sentence attempts to explain where transaction came from..

    Where you see transaction, assume this is the context that's relevant to your use, whether that be a Dataset or Transaction object.

    Would you prefer to remove that sentence and just create the transaction variable within the example?

  2. self-assigned this
    on Sep 30, 2015
  3. jgeewax commented on Sep 30, 2015

    @jgeewax
    ContributorAuthor

    Would you prefer to remove that sentence and just create the transaction variable within the example?

    Yea -- I suspect people will be lazy (like I just was) and read the code carefully, but not necessarily the surrounding documentation.

    I'd even be OK with something like:

    transaction = /* Read above about how to get the transaction */ null;

    But obviously working code would be better (particularly if it's just one line).

    The goal of testing our snippets was copy/paste shouldn't throw syntax errors... so having an undefined variable like transaction makes me think "whoops they probably made a mistake" not "hmm they probably explained that in some text above this". Does that make sense?

  4. callmehiphop commented on Sep 30, 2015

    @callmehiphop
    Contributor

    It totally makes sense. There actually is a transaction variable, but for whatever reason it's in the developer's documentation section.. so it never makes it to the site.

    https://github.com/GoogleCloudPlatform/gcloud-node/blob/master/lib/datastore/request.js#L92

  5. callmehiphop commented on Sep 30, 2015

    @callmehiphop
    Contributor

    Ah, so that particular file has a lot of references to transaction scattered throughout and we can only create transaction objects via callbacks, I'm assuming it was done this way to simplify the example. What are your feelings on something like..

    function getCompany(transaction, done) {
      var key = dataset.key(['Company', 123]);
    
      transaction.get(key, function(err, entity) {
        done();
      });
    }
    
    dataset.runInTransaction(getCompany, function(err) {});
  6. jgeewax commented on Oct 4, 2015

    @jgeewax
    ContributorAuthor

    Hmm.. Maybe it's worth noting that when I show up on the datastore.get() docs page, I'm expecting something like..

    var key = dataset.key(['Company', 1234]);
    dataset.get(key, function(err, entity) {  // is this even valid?
      // .. Do something with your entity :)
    });

    That is -- I'm not really interested in transactions right now... I just want to know "how do I retrieve a key from this service?"

    Since the docs are about dataset objects, I can assume that I already know how to get a dataset (if not, I go to the top and read the example there that shows how to grab a dataset).

  7. jgeewax commented on Oct 23, 2015

    @jgeewax
    ContributorAuthor

    Any update here ? Just went to look up the docs, saw the bug, went to master, saw the bug :(

  8. callmehiphop commented on Oct 23, 2015

    @callmehiphop
    Contributor

    Pushed fixes for the bugs into #903.

    As for the transaction vs dataset stuff, both are referencing the DatastoreRequest documentation, I can change it to dataset but then the transaction documentation will be wrong. A quick fix would be to add a second set of examples for dataset, does that sound ok?

  9. callmehiphop commented on Oct 24, 2015

    @callmehiphop
    Contributor
  10. stephenplusplus commented on Oct 24, 2015

    @stephenplusplus
    Contributor

    Sgtm

  11. 5 remaining items

  12. added a commit that references this issue on Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

api: datastoreIssues related to the Datastore API.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions