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 write and mWrite #1302

Merged
merged 17 commits into from May 28, 2019

Conversation

@Aschen
Copy link
Contributor

commented May 20, 2019

What does this PR do ?

(Open discussion)

This PR add two method for the bulk controller that behave like createOrReplace and mCreateOrReplace:

  • write: write a document without adding metadata or performing data validation
  • mWrite: write several documents without adding metadata or performing data validation

The name is open to debate, I have thought of put and mPut also

The goal is to have a low level method that allow to arbitrary write document into the storage layer to bypass automatic metadata or the data validation layer.

It's better to have these two endpoints than an option like bypassMeta on existing methods because developer can choose to not expose these methods to users.

How should this be manually tested?

  • Step 1 :
  • Step 2 :
  • Step 3 :
    ...

Other changes

  • Clone kuzzle configuration before passing it in the plugin context

@Aschen Aschen self-assigned this May 20, 2019

nit
@scottinet

This comment has been minimized.

Copy link
Member

commented May 20, 2019

@Aschen > wouldn't the bulk controller be more suited for such tasks, and easier to configure regarding rights management?
And in that regard, I would make real-time notifications optional (and off by default).

What do you think?

@Aschen

This comment has been minimized.

Copy link
Contributor Author

commented May 20, 2019

@scottinet Yes I agree, it's more logical to put this kind of low-level operation together!

I also agree for the notifications, I needed them for my usecase but again since these are low-level method It's better to let the developer choose what he want to do.

@codecov

This comment has been minimized.

Copy link

commented May 20, 2019

Codecov Report

Merging #1302 into 1-dev will decrease coverage by 0.38%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##            1-dev    #1302      +/-   ##
==========================================
- Coverage   93.84%   93.46%   -0.39%     
==========================================
  Files          98       98              
  Lines        6857     6885      +28     
==========================================
  Hits         6435     6435              
- Misses        422      450      +28
Impacted Files Coverage Δ
lib/services/elasticsearch.js 90.32% <0%> (-3.26%) ⬇️
lib/api/controllers/documentController.js 91.33% <0%> (-7.81%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a032190...f173e1f. Read the comment docs.

@codecov

This comment has been minimized.

Copy link

commented May 20, 2019

Codecov Report

Merging #1302 into 1-dev will decrease coverage by 0.15%.
The diff coverage is 81.13%.

Impacted file tree graph

@@            Coverage Diff             @@
##            1-dev    #1302      +/-   ##
==========================================
- Coverage   93.85%   93.69%   -0.16%     
==========================================
  Files          98       98              
  Lines        6895     6934      +39     
==========================================
+ Hits         6471     6497      +26     
- Misses        424      437      +13
Impacted Files Coverage Δ
lib/config/httpRoutes.js 100% <ø> (ø) ⬆️
lib/api/controllers/controller.js 100% <100%> (ø) ⬆️
lib/api/controllers/collectionController.js 100% <100%> (ø) ⬆️
lib/api/core/plugins/pluginContext.js 95.58% <100%> (ø) ⬆️
lib/services/elasticsearch.js 91.71% <63.63%> (-1.94%) ⬇️
lib/api/controllers/bulkController.js 94.11% <92%> (-5.89%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d06476...42f9263. Read the comment docs.

Show resolved Hide resolved lib/services/elasticsearch.js
Show resolved Hide resolved lib/services/elasticsearch.js Outdated
Show resolved Hide resolved lib/api/controllers/bulkController.js

Aschen added some commits May 21, 2019

DRY
@Aschen

This comment has been minimized.

Copy link
Contributor Author

commented May 22, 2019

@scottinet @thomasarbona @Yoann-Abbes @alexandrebouthinon You might want to review this PR, I added the tests

Aschen and others added some commits May 22, 2019

Show resolved Hide resolved lib/api/controllers/bulkController.js Outdated
}

return Boolean(flagValue);

This comment has been minimized.

Copy link
@scottinet

scottinet May 24, 2019

Member

this is not ensureBooleanFlag anymore, since you're now retrieving the value

another name must now be found, for instance tryGetBoolean?

also, the jsdoc is now outdated

Update lib/api/controllers/bulkController.js
Co-Authored-By: Sébastien Cottinet <scottinet@protonmail.com>

Aschen added some commits May 25, 2019

@Aschen Aschen merged commit 78b0a9b into 1-dev May 28, 2019

4 checks passed

LGTM analysis: JavaScript No new or fixed alerts
Details
codecov/project 93.69% (-0.16%) compared to 3d06476
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
sonarqube SonarQube reported no issues

@Aschen Aschen deleted the add-document-write branch May 28, 2019

scottinet added a commit to kuzzleio/documentation that referenced this pull request Jun 4, 2019

Add page about bulk:write and bulk:mwrite (#300)
## What does this PR do?

Documents bulk:write and bulk:mWrite.  
See kuzzleio/kuzzle#1302

### How should this be manually tested?


  - Step 1 :
  - Step 2 :
  - Step 3 :  
  ...

### Other changes

Add references to these methods in data validation, kuzzle metadata and real-time guides

@Aschen Aschen referenced this pull request Jun 14, 2019

Merged

Release 1.8.0 #1322

Aschen added a commit that referenced this pull request Jun 14, 2019

Merge pull request #1322 from kuzzleio/1.8.0-proposal
Release 1.8.0

Bug fixes

    [ #1311 ] Fix promise leaks (scottinet)
    [ #1298 ] Fix disabled protocol initialization (Aschen)
    [ #1297 ] Fix timeouts on plugin action returing the request (benoitvidis)
    [ #1288 ] Fix an error when trying a non-partial bulk update (scottinet)
    [ #1286 ] Allows bulk inserts on aliases (benoitvidis)
    [ #1282 ] Scan keys on redis cluster (benoitvidis)
    [ #1279 ] Users must be authenticated to use auth:logout (scottinet)

New features

    [ #1315 ] Add the new Vault module to handle encrypted application secrets (Aschen)
    [ #1302 ] Add write and mWrite (Aschen)
    [ #1305 ] Add pipes & hooks wildcard event (thomasarbona)

Enhancements

    [ #1318 ] Add a maximum ttl to auth:login (benoitvidis)
    [ #1301 ] Upgrade the WebSocket libraries (scottinet)
    [ #1308 ] Events triggering refactor (scottinet)
    [ #1300 ] Collection specifications methods cloisoned to a collection (thomasarbona)
    [ #1295 ] Improve validation error messages (benoitvidis)
    [ #1292 ] Throw an error when the realtime controller is invoked by plugin developers (benoitvidis)
    [ #1257 ] Add ability to define mapping policy for new fields (Aschen)
    [ #1291 ] Fix --help on subcommands (Yoann-Abbes)
    [ #1289 ] Handle ping/pong packets (scottinet)
    [ #1273 ] Fix incomplete access logs (scottinet)

Others

    [ #1317 ] Add ps dependency to plugin-dev Docker image for pm2 (benoitvidis)
    [ #1312 ] Check that .kuzzlerc.sample is well-formed (scottinet)
    [ #1299 ] Add Kuzzle Nightly & Redis 3 and 4 test (alexandrebouthinon)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.