Refactor: Split API modules into public/internal #1083
Conversation
mheinzel
force-pushed
the
mheinzel/split-api-public-private
branch
from
debbaf3 to
06da44a
Compare
services/gundeck/src/Gundeck/API.hs
Outdated
|
|
||
| Public.sitemap | ||
| Public.apiDocs | ||
| Internal.sitemap | ||
| head "/i/status" (continue $ const (return empty)) true |
There was a problem hiding this comment.
Why did you leave the /i/status out from the internal endpoints?
There was a problem hiding this comment.
They seemed very generic to me, in that every service has some common non-domain-related endpoints. Seems to me like they should be separated from the domain endpoints and could even be abstracted out for re-use (although in this case there's no big benefit for just 2 trivial endpoints).
There was a problem hiding this comment.
Also, in some services we don't have an Internal module, so we would have to create another module just for the status endpoint or it will be inconsistent.
There was a problem hiding this comment.
Also, in some services we don't have an
Internalmodule, so we would have to create another module just for the status endpoint or it will be inconsistent.
Where there are only a few endpoints, you could do the same as in some of brig's modules - routesPublic and routesInternal functions.
They seemed very generic to me, in that every service has some common non-domain-related endpoints. Seems to me like they should be separated from the domain endpoints and could even be abstracted out for re-use (although in this case there's no big benefit for just 2 trivial endpoints).
They are necessary and while yes, they are not serving business logic, but serve operational/deployment concerns, they still fall into the category of "internal" endpoints (since they are not publicly reachable). I was simply surprised to see why it might make sense to treat them differently, as I don't see a good reason to not place them into the internalRoutes or Internal.hs module along with the rest of /i/. If you keep them separated out, I think a comment explaining why they are treated differently is required, as a future developer (or a developer in the future) may wonder the same question - so I have this new endpoint, it starts with /i/something - now, should it be under Internal? Or not? Since some endpoints are there, but the /i/status is not, so which path should I follow with my endpoint?
There was a problem hiding this comment.
Okay, I'll move them to the other internal endpoints.
services not quite following the pattern:
cargohold:
All endpoints are public here, so I'm not sure it makes sense to create adonePublicsubmodule. Maybe it does, for consistency's sake?brig: The endpoints here are not in a single
APImodule, but there are some small ones as well. I didn't want to split each of these into multiple modules, so I just split them intoroutesPublicandroutesInternal.