feedhenry / fh-mbaas-api

Apache License 2.0
3 stars 35 forks source link

$fh.service expose full request API #21

Closed evanshortiss closed 6 years ago

evanshortiss commented 7 years ago

The $fh.service API has been around for a while now, and working on it with a wider group of consultants more recently, has resulted in some discussion about how it could possibly evolve. We're successfully using it in its current form, so these aren't really issues, but more of a nice to have.

  1. Can we expose the standard "request" module API?
  2. Remove the need to pass in a "guid" with every request
  3. Support "named services" so"guid"s are mapped to human friendly strings, e.g AUTH_SERVICE
  4. Simplify local development. Currently this uses FH_SERVICE_MAP, perhaps we can use an fhconfig.json file instead to determine where requests are routed? I've created a quick PoC module[1], ~180 sloc, that implements the above suggestions. It has a simple comparison of usage between it and $fh.service in the README [2], and tries something new for local development [3].

These changes could provide a better developer experience due to having the standard request API exposed (currently $fh.service is a subset) and an fhconfig,json file that's easy to read vs. FH_SERVICE_MAP. Arguably, it could produce cleaner code since each MBaaS is represented by an object rather than a series of characters in params to each service call, and you can use node streams easily (e.g weatherMbaas.get('/ireland/waterford').pipe(req)) to write more concise code.

Naturally this isn't backwards compatible, but could be a nice improvement. Thoughts @conoro @johnfriz @wei-lee?

[1] - https://github.com/evanshortiss/fh-service-request#usage [2] - https://github.com/evanshortiss/fh-service-request#fh-service-request-vs-fhservice [3] - https://github.com/evanshortiss/fh-service-request#local-development

wei-lee commented 7 years ago

Thanks @evanshortiss, I think these are excellent suggestions.

Can we expose the standard "request" module API? Remove the need to pass in a "guid" with every request

Yes, we certainly can. I think this is a much better format of APIs compared to our current one.

Support "named services" so"guid"s are mapped to human friendly strings, e.g AUTH_SERVICE

We are actually looking at this as well at the moment. We have a technical proposal about how to implement it in the platform. Feel free to read it and add comments. The main idea is that developers will be able to give names to the associations between services & projects, and use them in the $fh.service call instead of guid.

Simplify local development. Currently this uses FH_SERVICE_MAP, perhaps we can use an fhconfig.json file instead to determine where requests are routed?

I would say perhaps a different file, e.g. local-services.json. The reason being:

  1. we should make it clear it's for local development only,
  2. we only use fhconfig.json in the client apps, but not cloud apps. For now, I don't think we needed one just yet.
evanshortiss commented 7 years ago

Yes, we certainly can. I think this is a much better format of APIs compared to our current one.

$fh.service is not broken, it's just a little lacking in some neat features, so this is fantastic news.

would say perhaps a different file, e.g. local-services.json. The reason being:

Great point, that could be very confusing.

We are actually looking at this as well at the moment. We have a technical proposal about how to implement it in the platform. Feel free to read it and add comments.

Nice! Will take a look. FYI, this module implements some of the FH_SERVICE_MAP and guid lookup behaviour. Not sure how useful it will be, but it also has the hosts lookup. Will open a PR soon to $fh.host possibly using it. That's a while other discussion