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.

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

Fix document:search command #67

Merged
merged 6 commits into from
Oct 20, 2020
Merged

Fix document:search command #67

merged 6 commits into from
Oct 20, 2020

Conversation

Aschen
Copy link
Contributor

@Aschen Aschen commented Oct 14, 2020 •
edited
Loading

What does this PR do?

With the query hook, users are used to execute API action like this:

$ kourou <controller>:<action> <index> <collection> <body>

So I modified the document:search command be used with kourou document:search <index> <collection> <query>

Other changes

  • add fingerprint to api-key:search command display
  • avoid this.sdk?.<controller> usage

@scottinet
Copy link
Contributor

shouldn't we simply remove the document:search command, then? 🤔

@Aschen
Copy link
Contributor Author

Aschen commented Oct 14, 2020

The document:search command is still a "real" command because there is a few advantages to control the behavior:

  • better parameters (--from 10 instead of -a from=10)
  • better body: only the query without specifying query inside the body, the sort is passed into a flag (--sort)
  • real documentation of the command
  • better display of the documents

@Aschen Aschen self-assigned this Oct 14, 2020
@scottinet
Copy link
Contributor

scottinet commented Oct 14, 2020 •
edited
Loading

It seems that only how arguments are provided makes the difference. And I find that confusing. How, as a user, can I know in advance which one is executed, between document:search and... well... document:search?

I believe that we need to make additional commands stand alone more.

@Aschen
Copy link
Contributor Author

Aschen commented Oct 14, 2020

It seems that only how arguments are provided makes the difference. And I find that confusing. How, as a user, can I know in advance which one is executed, between document:search and... well... document:search?

I believe that we need to make additional commands stand alone more.

The hook only works for non-existing commands. If kourou doesn't know the command, then it try to pass it to sdk:query.

So the document:search command is always executed with the original command and never with the inhered one

@scottinet
Copy link
Contributor

I guess we'll discuss this in the pre-release workshop anyway. 🤔

@xbill82 xbill82 merged commit 0f93b49 into develop Oct 20, 2020
@xbill82 xbill82 deleted the fix-document-search branch October 20, 2020 13:21
@Aschen Aschen mentioned this pull request Oct 20, 2020
Merged
Aschen added a commit that referenced this pull request Oct 20, 2020
@Aschen
Merge pull request #68 from kuzzleio/0.15.0-proposal
36783ac 
# [0.15.0](https://github.com/kuzzleio/kourou/releases/tag/0.15.0) (2020-10-20)


#### New features

- [ [#61](#61) ] Add app:start-services command   ([Aschen](https://github.com/Aschen))
- [ [#63](#63) ] Add api-key support   ([rolljee](https://github.com/rolljee))

#### Enhancement

- [ [#67](#67) ] Fix document:search command   ([Aschen](https://github.com/Aschen))
- [ [#65](#65) ] Allows to infer body as first argument for query hook   ([Aschen](https://github.com/Aschen))

#### Others

- [ [#62](#62) ] Adds app:scaffold and app:run commands   ([Aschen](https://github.com/Aschen))
---
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scottinet scottinet scottinet approved these changes

@xbill82 xbill82 xbill82 approved these changes

Assignees

@Aschen Aschen

Labels
changelog:enhancements
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Aschen @scottinet @xbill82