pnxtech / hydra-express

A module which wraps Hydra and ExpressJS into a library for building distributed applications - such as microservices
MIT License
184 stars 37 forks source link

body-parser-config #85

Closed lfroe closed 6 years ago

lfroe commented 7 years ago

I tried adding the possibility to specify the body-parser configuration in the config.json file and then set it inside the initService-function.

lfroe commented 7 years ago

I incorporated your suggestions and also did basically the same thing for the cors-filter.

cjus commented 7 years ago

@cpraalufr @verg1l thanks. @emadum I'll add to the current experimental release for testing.

cjus commented 7 years ago

Published under hydra-express@1.4.8-experimental.3 We'll use that for further testing. Some thoughts on this though. I feel that we need a better way of allowing users to add and configure express middleware. @emadum in our own usecase at Flywheel that could be handled in the Flywheel plugin you created. For other uses they would be free to add what they need using a cleaner mechanism. @cpraalufr, @emadum what do you think?

lfroe commented 7 years ago

@cjus I think that the registerMiddlewareCallback-function already provides a pretty neat way to achieve this, because it lets the user handle things in the main-file, as he would in a basic express application. The only problem I see with this is the middleware that's registered in the initService-function and can't be overriden by the user. Therefore, in my opinion, a solution for this could be as simple as calling the registerMiddlewareCallback at the very start of the initService-function, thereby applying the more specific middleware - i.e. the stuff specified by the user inside the callback - first and only applying the necessary/missing middleware in the initService-function afterwards.

cjus commented 7 years ago

@lfroe can you rebase? I'd like to merge.

lukaszjanyga commented 6 years ago

Hi @cjus This fix is somehow non-existent in 1.5.5, how so? It is causing us serious issues as it's impossible to override.

cjus commented 6 years ago

@alvarg93 this is strange I did a git blame and it seems this was never merged. Do you have a hydra release where this exists? If not, I can issue an update to add this change.

lukaszjanyga commented 6 years ago

@cjus The one you mentioned above (hydra-express@1.4.8-experimental.3) does have this change.

cjus commented 6 years ago

@alvarg93 I understand now. The change was in an experimental version of HydraExpress for testing purposes but was probably never added to the main release. I'll do that now.

lukaszjanyga commented 6 years ago

@cjus Great! 🙂

cjus commented 6 years ago

@alvarg93 This is now published: hydra-express@1.6.3 See changes here: https://github.com/flywheelsports/hydra-express/pull/109/files

Please confirm that this is working for you.

lukaszjanyga commented 6 years ago

Just tested - works like a charm 🙂

cjus commented 6 years ago

@alvarg93 can you write a paragraph or two about how you're using it and what values you're setting and why? I'd like to add some documentation around this at https://www.hydramicroservice.com/ you can email me at cjus34@gmail.com