Skip to content

Rf/zenko 250 portmdsearchtest #1218

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 25, 2018
Merged

Rf/zenko 250 portmdsearchtest #1218

merged 1 commit into from
Apr 25, 2018

Conversation

@alexanderchan-scality
Copy link
Contributor

@alexanderchan-scality alexanderchan-scality commented Apr 18, 2018 •
edited
Loading

Documentation: #1220

philipyoo
philipyoo previously approved these changes Apr 18, 2018
View reviewed changes
tests/functional/aws-node-sdk/test/mdSearch/basicSearch.js
const userMetadata = { food: 'pizza' };
const updatedUserMetadata = { food: 'cake' };

runIfMongo('Basic search', () => {
Copy link
Contributor

@philipyoo philipyoo Apr 18, 2018

Choose a reason for hiding this comment

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

this is cool 👍

tests/functional/aws-node-sdk/test/mdSearch/versionEnabledSearch.js Outdated
Key: masterKey, Metadata: userMetadata },
err => {
// give ingestion pipeline some time
// setTimeout(() => done(err), 45000);
Copy link
Contributor

@philipyoo philipyoo Apr 18, 2018

Choose a reason for hiding this comment

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

just curious, why were the timeouts originally used? Is this the same as the timeout applied in the npm run command ft_search?

Copy link
Contributor Author

@alexanderchan-scality alexanderchan-scality Apr 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

The current implementation of MD search queries the mongo MD backend, and, as such, it does not require the wait time. (@rahulreddy confirm/elaborate this statement?)

setTimeout was originally used here when MD search was using Clueso. As for how it relates to the npm command timeout: the npm timeout is set to a larger number to make sure this timeout wait does not expire the test.

lib/api/apiUtils/bucket/validateSearch.js Outdated
try {
ast = parser.parse(searchParams);
ast = parser.parse(searchParams, (operator, operands) => {
switch (operator) {
Copy link
Contributor

@philipyoo philipyoo Apr 18, 2018

Choose a reason for hiding this comment

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

why not just an if/else?

Copy link
Contributor Author

@alexanderchan-scality alexanderchan-scality Apr 18, 2018

Choose a reason for hiding this comment

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

had more cases, but they turned out redundant. will clean up after more reviews

vrancurel
vrancurel previously approved these changes Apr 19, 2018
View reviewed changes
vrancurel
vrancurel previously approved these changes Apr 19, 2018
View reviewed changes
Copy link
Contributor

@vrancurel vrancurel left a comment

Choose a reason for hiding this comment

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

Can you please include a MD-Search.Design.Md document explaining the design + a basic usage of the API and the command line tool, including differences with SQL.

I am up for not having to support backticks for quoting variables containing dashes because it is error prone (cf demo)

@vrancurel vrancurel modified the milestone: 6.2.5 branch Apr 19, 2018
@alexanderchan-scality alexanderchan-scality force-pushed the rf/ZENKO-250-portmdsearchtest branch from bb06afa to 415afb0 Compare April 19, 2018 18:26
@ironman-machine ironman-machine dismissed vrancurel’s stale review April 19, 2018 18:26

Do it again human slave!:point_right: :runner: (Oh and the pull request has been updated, by the way.)

@ironman-machine
Copy link
Contributor

ironman-machine commented Apr 19, 2018

PR has been updated. Reviewers, please be cautious.

ssalaues
ssalaues previously approved these changes Apr 19, 2018
View reviewed changes
@alexanderchan-scality alexanderchan-scality force-pushed the rf/ZENKO-250-portmdsearchtest branch from 415afb0 to 53c6fc8 Compare April 19, 2018 18:43
@ironman-machine
Copy link
Contributor

ironman-machine commented Apr 19, 2018

PR has been updated. Reviewers, please be cautious.

@vrancurel
Copy link
Contributor

vrancurel commented Apr 19, 2018

and the design !

@alexanderchan-scality
Copy link
Contributor Author

alexanderchan-scality commented Apr 19, 2018

I have split the documentation into another PR #1220

@alexanderchan-scality alexanderchan-scality force-pushed the rf/ZENKO-250-portmdsearchtest branch from 53c6fc8 to 70868ff Compare April 20, 2018 06:55
@ironman-machine
Copy link
Contributor

ironman-machine commented Apr 20, 2018

PR has been updated. Reviewers, please be cautious.

@alexanderchan-scality alexanderchan-scality force-pushed the rf/ZENKO-250-portmdsearchtest branch from 70868ff to 5a6bba6 Compare April 20, 2018 06:58
@ironman-machine
Copy link
Contributor

ironman-machine commented Apr 20, 2018

PR has been updated. Reviewers, please be cautious.

@alexanderchan-scality alexanderchan-scality force-pushed the rf/ZENKO-250-portmdsearchtest branch from 5a6bba6 to 28c7a1b Compare April 20, 2018 07:34
@ironman-machine
Copy link
Contributor

ironman-machine commented Apr 20, 2018

PR has been updated. Reviewers, please be cautious.

@alexanderchan-scality alexanderchan-scality force-pushed the rf/ZENKO-250-portmdsearchtest branch from 28c7a1b to 628724e Compare April 20, 2018 17:34
@ironman-machine
Copy link
Contributor

ironman-machine commented Apr 20, 2018

PR has been updated. Reviewers, please be cautious.

philipyoo
philipyoo previously approved these changes Apr 20, 2018
View reviewed changes
@alexanderchan-scality alexanderchan-scality force-pushed the rf/ZENKO-250-portmdsearchtest branch from 628724e to 6d67e98 Compare April 24, 2018 23:11
@ironman-machine
Copy link
Contributor

ironman-machine commented Apr 24, 2018

PR has been updated. Reviewers, please be cautious.

@ironman-machine
Copy link
Contributor

ironman-machine commented Apr 24, 2018 

CONFLICT (add/add): Merge conflict in package.json
CONFLICT (add/add): Merge conflict in package-lock.json
CONFLICT (add/add): Merge conflict in lib/metadata/ModelVersion.md
CONFLICT (add/add): Merge conflict in lib/api/bucketPutLifecycle.js

@rahulreddy rahulreddy changed the base branch from master to z/1.0 April 25, 2018 00:43
@rahulreddy rahulreddy merged commit 9e1e00c into z/1.0 Apr 25, 2018
@rahulreddy rahulreddy deleted the rf/ZENKO-250-portmdsearchtest branch April 25, 2018 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rahulreddy rahulreddy rahulreddy approved these changes

+4 more reviewers

@vrancurel vrancurel vrancurel left review comments

@philipyoo philipyoo philipyoo left review comments

@bennettbuchanan bennettbuchanan bennettbuchanan left review comments

@ssalaues ssalaues ssalaues left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

prio-high

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@alexanderchan-scality @ironman-machine @vrancurel @rahulreddy @philipyoo @bennettbuchanan @ssalaues