Skip to content

feat: lazy load and hot reload for policies#890

Merged
XVincentX merged 17 commits intoExpressGateway:masterfrom
ravikp7:lazy-load-policies
Apr 8, 2019
Merged

feat: lazy load and hot reload for policies#890
XVincentX merged 17 commits intoExpressGateway:masterfrom
ravikp7:lazy-load-policies

Conversation

@ravikp7
Copy link
Copy Markdown
Contributor

@ravikp7 ravikp7 commented Mar 25, 2019

  • only load policies defined in the gateway config

fixes #870

@ravikp7 ravikp7 changed the title fix: lazy load policies [WIP] fix: lazy load policies Mar 26, 2019
@ravikp7 ravikp7 force-pushed the lazy-load-policies branch from 8c92276 to 038d5ab Compare March 26, 2019 23:24
@ravikp7 ravikp7 changed the title [WIP] fix: lazy load policies fix: lazy load policies Mar 26, 2019
@ravikp7 ravikp7 force-pushed the lazy-load-policies branch 3 times, most recently from 9de54d9 to d05f51d Compare March 26, 2019 23:40
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 26, 2019

Codecov Report

Merging #890 into master will increase coverage by 0.26%.
The diff coverage is 91.93%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #890      +/-   ##
==========================================
+ Coverage   88.82%   89.08%   +0.26%     
==========================================
  Files         136      137       +1     
  Lines        3695     3711      +16     
==========================================
+ Hits         3282     3306      +24     
+ Misses        413      405       -8
Impacted Files Coverage Δ
lib/policies/index.js 100% <100%> (ø) ⬆️
lib/policies/oauth2/oauth2.js 88.05% <100%> (+0.18%) ⬆️
lib/policies/basic-auth/basic-auth.js 100% <100%> (+7.69%) ⬆️
lib/config/config.js 90.42% <100%> (+0.1%) ⬆️
lib/policies/oauth2/index.js 100% <100%> (ø) ⬆️
lib/gateway/index.js 94.11% <86.95%> (-2.38%) ⬇️
lib/policies/basic-auth/registerStrategy.js 91.3% <91.3%> (ø)
lib/rest/index.js 91.11% <0%> (+7.39%) ⬆️
lib/conditions/json-schema.js 63.63% <0%> (+18.63%) ⬆️

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 91b62b0...dabeac1. Read the comment docs.

Copy link
Copy Markdown
Member

@XVincentX XVincentX left a comment

Choose a reason for hiding this comment

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

Good start.

I'd like to get a clarification on why you needed to load some policies in the tests although these are not used.

Another thing that needs to be verified is whether, by making an hot reload with a new policy, it gets loaded correctly and does not make the gateway crash.

Having a test checking this functionality would be great!

Good work though!

@ravikp7 ravikp7 force-pushed the lazy-load-policies branch from d9a7249 to de38388 Compare March 28, 2019 02:33
@ravikp7 ravikp7 changed the title fix: lazy load policies feat: lazy load and hot reload for policies Mar 28, 2019
@ravikp7 ravikp7 force-pushed the lazy-load-policies branch 3 times, most recently from 121d6f3 to 598f5be Compare March 28, 2019 20:26
ravikp7 added 2 commits April 5, 2019 19:46
- only load policies defined in the gateway config

fixes ExpressGateway#870
- add hot reload for policy changes in config file
- earlier, policies were not relaoded on hot reload
@XVincentX XVincentX force-pushed the lazy-load-policies branch from 598f5be to c30f31d Compare April 5, 2019 17:46
Copy link
Copy Markdown
Member

@XVincentX XVincentX left a comment

Choose a reason for hiding this comment

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

This looks promising, I can definitely see some progress. I've left another round of comments and I'm right now looking into the tests to figure out and fix the issue you were facing.

@XVincentX XVincentX force-pushed the lazy-load-policies branch from 6b88ea8 to 831cd98 Compare April 7, 2019 20:09
@ravikp7
Copy link
Copy Markdown
Contributor Author

ravikp7 commented Apr 8, 2019

@XVincentX Do we also need to remove policies which are removed from the config on hot reload?

@XVincentX
Copy link
Copy Markdown
Member

@ravikp7 No; they're on memory already so it does not make sense to unload them. In any case I took care of it and I'm adding an e2e test. See the code in case you're interested!

@XVincentX XVincentX merged commit d8db48e into ExpressGateway:master Apr 8, 2019
gatherchou pushed a commit to yilu-tech/express-gateway that referenced this pull request Jul 29, 2021
feat: lazy load and hot reload for policies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lazy load policies

3 participants