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

Introduce Search API #648

Merged
merged 2 commits into from
Jun 25, 2015
Merged

Conversation

stephenplusplus
Copy link
Contributor

Fixes #632

Docs: https://stephenplusplus.github.io/gcloud-node/#/docs/master/search

To dos:

  • Enable stream mode in Search#getIndexes()
  • Enable stream mode in Index#getDocuments()
  • Enable stream mode in Index#search()
  • Docs
    • JSDocs
    • Readme
    • Site
      • Update sitemap
      • Add "Search Overview" section
    • StreamRouter
  • Tests
    • Unit
    • System

API can't-do's:

  • Can't delete an index (can list indexes and delete all documents)
  • Can't create an index directly (only when creating a document)
  • Can't search a specific document

Not perfected yet:

  • Document JSON (when creating a document, we may be able to ease the field specification process)
    • See example below
  • Queries (we can probably have an object builder rather than just accepting a string)
    • It seems better to allow the developer to use the familiar query language as outlined in the docs. Our re-implementing the grammar could introduce inconsistencies and prevent portability.

Examples:

Create an index / Create a document from scratch

var search = gcloud.search();
var index = search.index('gcloud-services');

var document = index.document('search');
document.addField('implemented').addAtomValue('almost');

index.createDocument(document, function(err, document, apiResponse) {});

Create a document from an existing JSON object / file

var search = gcloud.search();
var index = search.index('gcloud-services');

var documentJson = {
  docId: 'search',
  fields: {
    values: [
      implemented: {
        stringValue: 'almost',
        stringFormat: 'ATOM'
      }
    ]
  }
};

index.createDocument(documentJson, function(err, document, apiResponse) {});

Get search results with a callback

var search = gcloud.search();
var index = search.index('gcloud-services');

function onSearchResults(err, documents, nextQuery, apiResponse) {
  if (err) {
    console.error(err);
    return;
  }

  // documents[0].id = 'search';

  if (nextQuery) {
    index.search(nextQuery, onSearchResults);
  }
}

index.search('implemented:almost', onSearchResults);

Stream search results

var search = gcloud.search();
var index = search.index('gcloud-services');

index.search('implemented:almost')
  .on('error', console.error)
  .on('data', function(document) {
    // document.id = 'search';
  });

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 8, 2015
deleteAllDocuments(done);
});

it('should create a document in a new index', function(done) {

This comment was marked as spam.

@stephenplusplus
Copy link
Contributor Author

Index#getDocuments and Index#search are now stream-able.

Also, I added document building functionality and updated the first post here with some examples.

@stephenplusplus
Copy link
Contributor Author

The todo list is a-ticking away. Now would be a good time to catch any mis-steps (see the docs). Next to tackle are the tests.

@stephenplusplus stephenplusplus force-pushed the spp--search branch 8 times, most recently from 9646049 to 1aa9f1e Compare June 17, 2015 13:12
@stephenplusplus
Copy link
Contributor Author

This is ready for review (from who, I don't know 😄). Todos completed.

@ryanseys
Copy link
Contributor

I will
On Jun 17, 2015 6:13 AM, "Stephen Sawchuk" notifications@github.com wrote:

This is ready for review (from who, I don't know [image: 😄]). Todos
completed.


Reply to this email directly or view it on GitHub
#648 (comment)
.


/*! Developer Documentation
*
* streamRouter is used to extend `nextQuery`+callback methods with stream

This comment was marked as spam.

@ryanseys
Copy link
Contributor

I run npm test and get the following console logs at the end of the test results (no failures):

{ [Error: ENOENT, open '/path/to/keyfile.json'] errno: -2, code: 'ENOENT', path: '/path/to/keyfile.json' }
{ [Error: ENOENT, open '/path/to/keyfile.json'] errno: -2, code: 'ENOENT', path: '/path/to/keyfile.json' }
{ [Error: ENOENT, open '/path/to/keyfile.json'] errno: -2, code: 'ENOENT', path: '/path/to/keyfile.json' }

@stephenplusplus
Copy link
Contributor Author

I thought that might just be me. Can you advise how to debug that?

@ryanseys
Copy link
Contributor

I think it's because we pass the external console object into the vm object when we test our code docs, so if the docs that are tested have any calls to console.log or console.error then they will be written to our external stdout. This is the case with some of your tests when you run .on('error', console.error).

Easiest way to fix is overwrite console.log and console.error for the docs test.

});

describe('search', function() {
var query = 'ryan';

This comment was marked as spam.

@@ -78,6 +78,7 @@ describe('documentation', function() {
gcloud: gcloud,
require: require,
process: process,
console: console,

This comment was marked as spam.

This comment was marked as spam.

@@ -74,10 +74,16 @@ describe('documentation', function() {
].join('\n'));
}

var mockConsole = Object.keys(console).reduce(function(console, method) {

This comment was marked as spam.

This comment was marked as spam.

@ryanseys
Copy link
Contributor

Looks GREAT to me!

@stephenplusplus stephenplusplus force-pushed the spp--search branch 2 times, most recently from 0e7e32c to fc3cf78 Compare June 17, 2015 18:37
*
* var document = search.index('records').document('stephen');
*
* //-

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@stephenplusplus
Copy link
Contributor Author

Wee! I just added this in to handle pre-mature closing of a stream: stephenplusplus@abca6f1

Basically to handle something like this:

index.getDocuments().on('data', function(document) {
  if (document.id === 'the-one-i-want') {
    this.end();
  }
});

Before the change, we would have had no idea end was called, and would have kept trying to push documents to the stream & even continue calling the API for more results.

@stephenplusplus
Copy link
Contributor Author

Merge-time!

stephenplusplus added a commit that referenced this pull request Jun 25, 2015
@stephenplusplus stephenplusplus merged commit 61ffa82 into googleapis:master Jun 25, 2015
chingor13 pushed a commit that referenced this pull request Aug 24, 2022
Source-Link: googleapis/synthtool@d229a12
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:74ab2b3c71ef27e6d8b69b1d0a0c9d31447777b79ac3cd4be82c265b45f37e5e
sofisl pushed a commit that referenced this pull request Nov 11, 2022
Source-Link: googleapis/synthtool@d229a12
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:74ab2b3c71ef27e6d8b69b1d0a0c9d31447777b79ac3cd4be82c265b45f37e5e
sofisl pushed a commit that referenced this pull request Jan 24, 2023
…ncy versions (#648)

This PR was generated using Autosynth. 🌈

Synth log will be available here:
https://source.cloud.google.com/results/invocations/9be7b892-4bc6-4dcb-8dc8-41f27e5fc193/targets

- [ ] To automatically regenerate this PR, check this box.

Source-Link: googleapis/synthtool@fdd03c1
sofisl pushed a commit that referenced this pull request Jan 25, 2023
…ncy versions (#648)

This PR was generated using Autosynth. 🌈

Synth log will be available here:
https://source.cloud.google.com/results/invocations/9be7b892-4bc6-4dcb-8dc8-41f27e5fc193/targets

- [ ] To automatically regenerate this PR, check this box.

Source-Link: googleapis/synthtool@fdd03c1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants