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

KZL-174 Expose to plugins a SDK instance directly mapped to the funnel #1179

Merged
merged 16 commits into from Sep 13, 2018

Conversation

Projects
None yet
5 participants
@ballinette
Member

ballinette commented Aug 10, 2018

What does this PR do ?

Apply https://jira.kaliop.net/browse/KZL-176 and https://jira.kaliop.net/browse/KZL-177

This creates a custom light network wrapper for SDK that is directly mapped to the funnel.
And exposes a SDK instance with this wrapper to the plugins.

How should this be manually tested?

Create a plugin and add hooks or custom routes which call context.accessors.sdk.<somectrl>.<someaction>

Other changes

  • funnel.executePluginRequest now returns a promise (no need to use callbacks anymore since we disabled the overload protection for this method)
  • deprecate context.accessors.execute method (will be removed in V2)

Boyscout

fix some jsdocs

Show outdated Hide outdated package.json Outdated
@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Aug 10, 2018

Codecov Report

Merging #1179 into 1-dev will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##            1-dev    #1179      +/-   ##
==========================================
+ Coverage   93.92%   93.97%   +0.04%     
==========================================
  Files          94       95       +1     
  Lines        6538     6541       +3     
==========================================
+ Hits         6141     6147       +6     
+ Misses        397      394       -3
Impacted Files Coverage Δ
lib/api/core/sdk/funnelProtocol.js 100% <100%> (ø)
lib/api/controllers/funnelController.js 95.38% <100%> (-0.25%) ⬇️
lib/api/core/plugins/pluginContext.js 95.86% <100%> (+2.76%) ⬆️

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 ae167ef...c22274f. Read the comment docs.

codecov-io commented Aug 10, 2018

Codecov Report

Merging #1179 into 1-dev will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##            1-dev    #1179      +/-   ##
==========================================
+ Coverage   93.92%   93.97%   +0.04%     
==========================================
  Files          94       95       +1     
  Lines        6538     6541       +3     
==========================================
+ Hits         6141     6147       +6     
+ Misses        397      394       -3
Impacted Files Coverage Δ
lib/api/core/sdk/funnelProtocol.js 100% <100%> (ø)
lib/api/controllers/funnelController.js 95.38% <100%> (-0.25%) ⬇️
lib/api/core/plugins/pluginContext.js 95.86% <100%> (+2.76%) ⬆️

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 ae167ef...c22274f. Read the comment docs.

@Aschen

This comment has been minimized.

Show comment
Hide comment
@Aschen

Aschen Aug 10, 2018

Contributor

What about the realtime capabilities of the SDK ?

Contributor

Aschen commented Aug 10, 2018

What about the realtime capabilities of the SDK ?

@ballinette

This comment has been minimized.

Show comment
Hide comment
@ballinette

ballinette Aug 10, 2018

Member

@Aschen I don't see how using realtime is relevant inside a plugin (which actions are scoped inside a client's request)
If you need to subscribe to filters and keep connected to kuzzle, you do not need a plugin, you need an external client.

Member

ballinette commented Aug 10, 2018

@Aschen I don't see how using realtime is relevant inside a plugin (which actions are scoped inside a client's request)
If you need to subscribe to filters and keep connected to kuzzle, you do not need a plugin, you need an external client.

@ballinette ballinette added the wip label Aug 10, 2018

@ballinette ballinette removed the wip label Aug 16, 2018

@Aschen

Aschen approved these changes Aug 17, 2018

@scottinet

I'm not at ease with that one. I mean, I obviously agree with the idea itself, but I believe it's too soon: we're adding an experimental and as-of-now undocumented SDK to plugin interfaces, so this is pretty much unusable. 😕

reverting to a comment

@ballinette

This comment has been minimized.

Show comment
Hide comment
@ballinette

ballinette Aug 22, 2018

Member

@scottinet I agree that it is quite soon for this feature, and of course it can wait for the SDK 6.0 release

However:

  • the sdk refacto is almost done and is just waiting for the documentation to be released
  • it could be fine to have this feature before the V2 to let early developers test it.

A suggestion could be: release the feature, but tag it as "experimental" for now. (and of course remove back the "deprecated" tag on context.accessors.execute)

This way we will be able to use it and test it for demos, POCs, etc, and some "pilote" projects could also try it and give their feedback.

Member

ballinette commented Aug 22, 2018

@scottinet I agree that it is quite soon for this feature, and of course it can wait for the SDK 6.0 release

However:

  • the sdk refacto is almost done and is just waiting for the documentation to be released
  • it could be fine to have this feature before the V2 to let early developers test it.

A suggestion could be: release the feature, but tag it as "experimental" for now. (and of course remove back the "deprecated" tag on context.accessors.execute)

This way we will be able to use it and test it for demos, POCs, etc, and some "pilote" projects could also try it and give their feedback.

@ballinette ballinette added the wip label Aug 22, 2018

@ballinette

This comment has been minimized.

Show comment
Hide comment
@ballinette

ballinette Aug 22, 2018

Member

put in "WIP" as we will wait for a beta release of SDK 6 to merge this PR

Member

ballinette commented Aug 22, 2018

put in "WIP" as we will wait for a beta release of SDK 6 to merge this PR

@Aschen Aschen changed the base branch from 1.x to 1-dev Sep 4, 2018

Aschen and others added some commits Sep 4, 2018

@scottinet scottinet removed the wip label Sep 11, 2018

@scottinet scottinet merged commit 895b449 into 1-dev Sep 13, 2018

4 checks passed

LGTM analysis: JavaScript No alert changes
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
sonarqube SonarQube reported no issues

@scottinet scottinet deleted the KZL-177-funnel-network-wrappper branch Sep 13, 2018

@scottinet scottinet referenced this pull request Sep 13, 2018

Merged

Release 1.5.0 #1194

scottinet added a commit that referenced this pull request Sep 13, 2018

Release 1.5.0 (#1194)
* fixes #1125 - aggs & sort on user search (#1144)

* Add AdminController (#1129)

* KZL-143 KZL-145 add admin controller and reset cache action

* Boyscout: fix crash because kuzzle try to load protocols in simplefiles

* Add tests for adminController

* Add resetKuzzleData and resetSecurity

* fix features for admin controller

* Add resetDatabase

* fix tests

* fix tests

* fix tests

* Add default rights restrictions for admin controller

* Fix test

* Fix test

* Use API for all actions except dump

* Add generateDump

* Fix tests and remove old cli tests

* Remove reset security scenario

* Make sonarqube happy

* Remove references to cliController

* Fix features

* fix internal error

* remove internal broker

* re-add shutdown

* fix test

* Put shutdown in adminController

* Reset defaults roles and profiles with resetSecurity

* replace ws by uws

* Nitpicking

* increase coverage

* increase coverage

* increase coverage

* fix linter

* increase coverage

* put back proxyBroker

* fix tests

* remove tests

* fix tests

* nitpicking

* Updates for PR

* Put back defaut config

* Disable dump if it is specified in config

* fix message in cli

* add truncate() method on repositories + refactor admin controller to use truncate()

* fix admin controller

* fix test

* Add Janitor core component

* return number of deleted objects in truncate

* fix linter

* catch TypeError for scanStream

* fix linter

* Please sonarqube

* fix linter

* Nitpicking

* Updates for PR

* update headers

* fix linter

* fix tests

* deprecate "constructor" auth option and replace it with "authenticator" (#1145)

* deprecate "constructor" auth option and replace it with "authenticator"

* fix sonarqube issues

* Add action and controller in request input for http events (#1143)

* Add action and controller in request input for http events

* put back some code

* nitpicking

* Add forgotten files --'

* Test Kuzzle against different node LTS versions (#1147)

* set ulimits for elasticsearch (#1148)

* Update kuzzle-common-objects module (#1150)

* adapt tests to new common objects version

* dependencies update

* npm audit fix

* update to plugin auth local 5.1.3 (#1149)

* update to plugin auth local 5.1.3

* adapt tests to new common objects version

* dependencies update

* npm audit fix

* prevent a breaking change when upgrade from a pre-1.4.0 version

* Revert "prevent a breaking change when upgrade from a pre-1.4.0 version"

This reverts commit 8328f50.

* prevent a breaking change occuring when upgrading Kuzzle with old config files (#1154)

* Inject protocol informations into the RequestContext object (#1155)

* inject protocol info into requestcontext

* fix incoming context infos coming from the proxy

* revert debug modification

* no need to manually copy incoming headers

* update unit tests

* fix sonarqube issu

* jsdoc fix

* fixes #1146, passport login request missing headers (#1153)

* fixes #1146, passport login request missing headers

* oauth - add some comments on arbritraty method & url

* Add script to build and push docker images (#1152)

* Add script to build and push docker images

* Add script to build and push docker images

* Fix build script

* add comment to secret env

* Use kuzzleteam account for dockerhub

* Update subnmodule

* Dont build and push on hotfix merge

* fix #1161 (#1162)

* fix startup message (#1163)

* Fix #1131: listCollections limited to the provided collection name, when specified (#1157)

* fixes #1156 - missing version in swagger output (#1164)

* add token infos to login response (#1171)

* disable dump by default (#1172)

* disable dump by default

* fix tests:

* fix #447: allow to search document with empty query through a HTTP GET request

* fix unit test

* Replace ParseError errors with BadRequestError objects (#1158)

* replace ParseError (deprecated) with BadRequestError

* fix #1168: remove useless CLI --port option  (#1177)

* fix #1168

* remove useless --port option in CLI start command

* fix #1174 (#1176)

* Fix #1115: Standardize the scrollId return value (#1173)

* Standardize the scrollId return value

* typo

* typo

* Initialize repositories before plugins (#1167)

* Initialize repositories before plugins

* remove one then() level

* Use local multi-stage build for kuzzleio/plugin-dev and kuzzleio/kuzzle (#1178)

* Update Dockerfile and build-docker-images script\n Now with multi-stage build and support for 1.x and 2.x development branch
* Remove pm2 from prod image
* Use latest node 8 version

* Add metadata to published documents (#1186)

* KZL 454 - Allow subscribe filters on metadata (#1185)

* Do not cache requests with publish(), call koncorde two times instead

* Add functionnal test for subscription filter on metadata

* Add LGTM.com code quality badges (#1187)

* KZL 290 - Add fixtures for securities (#1182)

* Move mappings and fixtures loading to Janitor

* Fix bug in repository#delete when object is not present

* Add ability to load roles, profiles and users to Janitor and CLI

* Add test for Janitor#loadSecurities

* remove .only from test

* Fix #1184: Better error message when object not found in repositories
Refactor repository loadOneFromDatabase method to reject with an explicit 404 when not found
Allow the old api usage which return a null object when not found for SecurityController#persistUser and PluginRepository#load

* Fix test

* rename getUserId

* Update for PR

* refactor janitor _create* method

* Please sonarqube

* typo

* Plugins/Protocols manifest.json overhaul (#1181)

* wip

* change reporter to dot

* fix polluting console outputs

* upgrade unit tests to make them handle the new plugin structure

* new manifest.json handling tests+fixes

* fix sonarqube issue

* x_x

* implement @benoitvidis asked changes

* fix circular json in manifest objects

* add control to manifest stringification

* remove privileged mode for protocols

* Update docker build script for new branch name 1-dev (#1190)

* Update images build script

* push only if we are on the major build

* remove test

* update default plugins (#1193)

* KZL-174 Expose to plugins a SDK instance directly mapped to the funnel (#1179)

* wip

* KZL-177: custom "funnel" network wrapper for SDK

* KZL-178: Export javascript SDK to plugins

* refactor executePluginRequest: now returns a promise

* KZL-177: revert funnelcallController => now sdk wrapper calls directly funnel.executePluginRequest

* mark `pluginContext.accessor.execute` as deprecated

* clean code

* update package-lock

* fix comment

* tix typo

* dependencies update

* remove debug config

* Release 1.5.0

* dependencies update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment