Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add datastore v1beta3 snippets #16

Merged
merged 1 commit into from
Nov 13, 2015

Conversation

stephenplusplus
Copy link
Contributor

// @pcostell

To Dos

  • Entities
  • Queries
  • Indexes
  • Transactions
  • Metadata

This is some snippet code in preparation of the datastore v1beta3 release. It's a work in progress, so I wanted to get reviews and guidance on the structure to follow as early as possible.

Some questions:

  • Do we have freedom to use taskKey (convention) vs task_key (snippet spreadsheet) - the linter caught this as an error, and I wasn't sure if either is a hard requirement.
  • gcloud-node doesn't have an Entity type; data that goes or comes from an entity are just plain JavaScript objects, without special capabilities
  • I assume this JavaScript snippet file is meant to be runnable-- as in, instead of dumping out a bunch of vars and function calls, I should box them inside of functions that can be called individually?

Any insight into how the docs are generated from this file, requirements or tips for conventions to follow would be helpful to make sure I get this right. Thanks!

@pcostell
Copy link
Contributor

  • taskKey is perfectly fine in this context, whatever is language idiomatic.
  • For the case of snippets like basic_entity, just show creating a javascript object with the appropriate fields.
  • Yes, all of our snippets are runnable (and do basic smoke tests to make sure the snippet is correct).

Snippets are handled using different regions. So for the basic_entity region this could look in code something like:

function testBasicEntity() {
  // [START basic_entity]
  var task = { .... };
  // [END basic_entity]
  datastore.put(task); // Just a smoke test to make sure the test doesn't fail.
}

One thing to note about the snippet is that is removes the appropriate whitespace, so don't have to worry about the snippets being at a top level.

In general, if an operation is not possible either provide a comment with a workaround or a comment that says it's not possible. For example some of our clients don't expose the insert command, so instead we show a transaction that does a get, verifies that it fails, then performs a put.

// [END Consistency:entity_with_parent]

// [START Entities:properties]
var taskData = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just call these task.

@stephenplusplus
Copy link
Contributor Author

Thanks for the review! I pushed some updates, let me know if I'm headed in the right direction. (updated first post with todos - feel free to add on)

@@ -11,6 +11,7 @@
"license": "Apache 2",
"private": true,
"dependencies": {
"gcloud": "stephenplusplus/gcloud-node#spp--datastore-v1beta3",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(temporary until release: googleapis/google-cloud-node#897)

type: 'Personal',
done: false,
priority: 4,
percent_complete: 10.0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's be ok here to do things in a language idomatic way and call this percentComplete.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, other node.js + datastore samples use camelCase.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this will change the indexes though, which are currently:

indexes:
- kind: Task
  properties:
  - name: done
  - name: priority
    direction: desc
- kind: Task
  properties:
  - name: priority
  - name: percent_complete
- kind: Task
  properties:
  - name: type
  - name: priority
- kind: Task
  properties:
  - name: priority
  - name: created
- kind: Task
  properties:
  - name: done
  - name: created
- kind: Task
  properties:
  - name: type
  - name: priority
    direction: desc
- kind: Task
  properties:
  - name: type
  - name: created

I was planning to include that file in this repo, but wherever it goes the percent_complete will have to be changed to match if we camelcase it.

@pcostell
Copy link
Contributor

So far the entity stuff LGTM

@stephenplusplus
Copy link
Contributor Author

The query files are up, please let me know if things look good.

@stephenplusplus stephenplusplus force-pushed the master branch 3 times, most recently from 67bc340 to fe4f73a Compare October 19, 2015 18:06
}
};

datastore.insert(task, function(err) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to insert on this snippet.

stephenplusplus added a commit to stephenplusplus/nodejs-docs-samples that referenced this pull request Oct 19, 2015
stephenplusplus added a commit to stephenplusplus/nodejs-docs-samples that referenced this pull request Oct 19, 2015
stephenplusplus added a commit to stephenplusplus/nodejs-docs-samples that referenced this pull request Oct 19, 2015
@stephenplusplus
Copy link
Contributor Author

@pcostell thanks! I've made the requested changes in the last few commits.

@stephenplusplus
Copy link
Contributor Author

All the sections are covered now.

propertiesByKind[kind].push(propertyName);
});

// propertiesByKind = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should build (and the other comments) in a language-agnostic way into the docs around the snippets, so no need to include them here.

NimJay pushed a commit that referenced this pull request Nov 11, 2022
ace-n pushed a commit that referenced this pull request Nov 11, 2022
ace-n pushed a commit that referenced this pull request Nov 11, 2022
ace-n pushed a commit that referenced this pull request Nov 11, 2022
ace-n pushed a commit that referenced this pull request Nov 14, 2022
ace-n pushed a commit that referenced this pull request Nov 15, 2022
ace-n pushed a commit that referenced this pull request Nov 15, 2022
ace-n pushed a commit that referenced this pull request Nov 17, 2022
ace-n pushed a commit that referenced this pull request Nov 17, 2022
ahrarmonsur pushed a commit that referenced this pull request Nov 17, 2022
kweinmeister pushed a commit that referenced this pull request Nov 18, 2022
* chore: lock files maintenance

* chore: lock files maintenance
pattishin pushed a commit that referenced this pull request Nov 18, 2022
pattishin pushed a commit that referenced this pull request Nov 22, 2022
jsimonweb pushed a commit to jsimonweb/nodejs-docs-samples that referenced this pull request Dec 12, 2022
telpirion pushed a commit that referenced this pull request Jan 11, 2023
🤖 I have created a release *beep* *boop*
---


## [0.1.1](googleapis/nodejs-video-stitcher@v0.1.0...v0.1.1) (2022-06-23)


### Bug Fixes

* **deps:** update dependency google-gax to v3 ([#15](googleapis/nodejs-video-stitcher#15)) ([345085f](googleapis/nodejs-video-stitcher@345085f))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
irataxy pushed a commit that referenced this pull request Jan 13, 2023
🤖 I have created a release *beep* *boop*
---


## [0.1.1](googleapis/nodejs-video-stitcher@v0.1.0...v0.1.1) (2022-06-23)


### Bug Fixes

* **deps:** update dependency google-gax to v3 ([#15](googleapis/nodejs-video-stitcher#15)) ([345085f](googleapis/nodejs-video-stitcher@345085f))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
pattishin added a commit that referenced this pull request Dec 13, 2023
# This is the 1st commit message:

refactor: updating codeowners

# This is the commit message #2:

add chat functions

# This is the commit message #3:

use correct testing project

# This is the commit message #4:

refactor: adding system tests + updating corresponding chat samples

# This is the commit message #5:

add countTokens sample

# This is the commit message #6:

refactor: adding in region tags, abstracting out mimetype, adding new image ur

# This is the commit message #7:

refactor: updating gs url in test, fix to args getting passed to sample functions

# This is the commit message #8:

refactor: resolving file paths in tests, adding wait helper function

# This is the commit message #9:

add warning about safety concerns

# This is the commit message #10:

refactor:filling out nonstreamingchat and streamcontent tests

# This is the commit message #11:

add countTokens test

# This is the commit message #12:

refactor: filling out more streaming tests

# This is the commit message #13:

add safety settings test

# This is the commit message #14:

refactor: adding in stream content and multipart content tests

# This is the commit message #15:

feat: add new sendMultiModalPromptWithImage sample

# This is the commit message #16:

refactor: adding region tags
pattishin added a commit that referenced this pull request Dec 13, 2023
* feat: initial base for generative-ai

* # This is a combination of 16 commits.
# This is the 1st commit message:

refactor: updating codeowners

# This is the commit message #2:

add chat functions

# This is the commit message #3:

use correct testing project

# This is the commit message #4:

refactor: adding system tests + updating corresponding chat samples

# This is the commit message #5:

add countTokens sample

# This is the commit message #6:

refactor: adding in region tags, abstracting out mimetype, adding new image ur

# This is the commit message #7:

refactor: updating gs url in test, fix to args getting passed to sample functions

# This is the commit message #8:

refactor: resolving file paths in tests, adding wait helper function

# This is the commit message #9:

add warning about safety concerns

# This is the commit message #10:

refactor:filling out nonstreamingchat and streamcontent tests

# This is the commit message #11:

add countTokens test

# This is the commit message #12:

refactor: filling out more streaming tests

# This is the commit message #13:

add safety settings test

# This is the commit message #14:

refactor: adding in stream content and multipart content tests

# This is the commit message #15:

feat: add new sendMultiModalPromptWithImage sample

# This is the commit message #16:

refactor: adding region tags

* refactor: updating codeowners

add chat functions

use correct testing project

refactor: adding system tests + updating corresponding chat samples

add countTokens sample

refactor: adding in region tags, abstracting out mimetype, adding new image ur

refactor: updating gs url in test, fix to args getting passed to sample functions

refactor: resolving file paths in tests, adding wait helper function

add warning about safety concerns

refactor:filling out nonstreamingchat and streamcontent tests

add countTokens test

refactor: filling out more streaming tests

add safety settings test

refactor: adding in stream content and multipart content tests

feat: add new sendMultiModalPromptWithImage sample

refactor: adding region tags

update to common prompt

fix: resolve linting

* refactor: remove index file

---------

Co-authored-by: [email protected] <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants