Skip to content
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

Refactor API servers into one modular server #60

Merged
merged 66 commits into from
Sep 13, 2018
Merged

Refactor API servers into one modular server #60

merged 66 commits into from
Sep 13, 2018

Conversation

eemeli
Copy link
Member

@eemeli eemeli commented Sep 8, 2018

Fixes #57

This PR effects a number of related changes:

  • kansa is renamed as server (database schema is still kansa, mind), and becomes the only remaining front-facing express.js server. Relatively few code changes except for the loading of optional modules:
    Object.keys(config.modules).forEach(name => {
      const mc = config.modules[name]
      if (mc) {
        const module = require(`./modules/${name}`)
        app.use(`/${name}`, module(pgp, mc))
      }
    })
  • hugo and raami are each refactored as modules, and moved under modules/
  • nginx is renamed as proxy
  • A modules section is added to config/kansa.yaml

Actual implementation and functionality should not be affected by any of these changes, and there are no changes to the database. A few things that should belong to individual modules don't yet; specifically the tests for Hugo nomination endpoints are still under integration-tests/.

@eemeli
Copy link
Member Author

eemeli commented Sep 8, 2018

Updates:

  • The Slack inviter is now also a module slack which takes & requires a few config parameters
  • npm install is run for enabled modules
  • Error objects are now under /common/errors/ and are published as @kansa/errors

@offbyone
Copy link

offbyone commented Sep 8, 2018

I'm late to the party, but I wanted to add that I actually really prefer the microserver architecture that is in there now to this. It gives the freedom to break out parts of the functionality of the system and replace them with other tools in a modular fashion behind the main API facade. That's a really powerful pattern, and I'd rather keep it.

@offbyone
Copy link

offbyone commented Sep 8, 2018

I want to be quite specific here, I want the ability to plug in different API frameworks, all the way up to language runtimes, under the main /api facade.

@eemeli
Copy link
Member Author

eemeli commented Sep 8, 2018

This change should in fact make it easier to implement services with other language runtimes, as it's starting a process of breaking apart the functionalities provided in particular by what was kansa into smaller modules, which may then in turn be enabled or disabled. No changes are made to e.g. persistence.

As a specific example, stripe is now a module alongside hugo and raami because the overhead involved in splitting into a module (mostly in terms of complexity) is now sufficiently low that it makes sense. To now use an alternative implementation for it, it suffices to disabled the express.js module from the kansa.yaml config, add the alternative to your container orchestration, and configure your reverse proxy for it. Previously doing the same would have been somewhat messier.

@offbyone
Copy link

offbyone commented Sep 8, 2018 via email

@eemeli
Copy link
Member Author

eemeli commented Sep 13, 2018

So as it turns out I didn't let this rest for a while.

  • Server file structure is pretty thoroughly refactored
  • The public module is added
  • Module init gets a new shared config parameter
  • @kansa/common version 1.1.0 is published on npm
  • Commit 9a7abfa fixes a parameter length limit in the Postgres function name_match()

I'm going to now merge this into master, and effectively continue development in a new dev branch that builds on top of this (including PRs #61 and #63) and includes changes to the API endpoints.

@eemeli eemeli merged commit 22a9894 into master Sep 13, 2018
@eemeli eemeli deleted the repack branch September 13, 2018 12:18
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.

2 participants