From 14adddd5660e0b0becb91ecaf9ae63b4c5725506 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Wed, 15 Sep 2021 15:33:42 +0100 Subject: [PATCH 1/3] Extract content about new workers into own doc This is a bit too niche for the README, which should be focussed on the bare minimum someone needs to know to get started with a repo. Moving this content to its own doc is consistent with other apps [1] and gives it more room to grow. [1]: https://github.com/alphagov/notifications-admin/tree/master/docs --- README.md | 26 ++------------------------ docs/creating-a-new-worker-app.md | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 24 deletions(-) create mode 100644 docs/creating-a-new-worker-app.md diff --git a/README.md b/README.md index 2c7467bc5..064ef1023 100644 --- a/README.md +++ b/README.md @@ -118,28 +118,6 @@ cf run-task notify-api "flask command purge_functional_test_data -u make cf-push` - -Once this is done, you can push your deployment changes to jenkins to have your app deployed on every deployment. +- [Creating a new worker app](docs/creating-a-new-worker-app.md) diff --git a/docs/creating-a-new-worker-app.md b/docs/creating-a-new-worker-app.md new file mode 100644 index 000000000..958594578 --- /dev/null +++ b/docs/creating-a-new-worker-app.md @@ -0,0 +1,24 @@ +# To create a new worker app + +You need to: +1. Create new entries for your app in `manifest.yml.j2` and `scripts/paas_app_wrapper.sh` ([example](https://github.com/alphagov/notifications-api/pull/2486/commits/6163ca8b45813ff59b3a879f9cfcb28e55863e16)) +1. Update the jenkins deployment job in the notifications-aws repo ([example](https://github.com/alphagov/notifications-aws/commit/69cf9912bd638bce088d4845e4b0a3b11a2cb74c#diff-17e034fe6186f2717b77ba277e0a5828)) +1. Add the new worker's log group to the list of logs groups we get alerts about and we ship them to kibana ([example](https://github.com/alphagov/notifications-aws/commit/69cf9912bd638bce088d4845e4b0a3b11a2cb74c#diff-501ffa3502adce988e810875af546b97)) +1. Optionally add it to the autoscaler ([example](https://github.com/alphagov/notifications-paas-autoscaler/commit/16d4cd0bdc851da2fab9fad1c9130eb94acf3d15)) + +**Important:** + +Before pushing the deployment change on jenkins, read below about the first time deployment. + +## First time deployment of your new worker + +Our deployment flow requires that the app is present in order to proceed with the deployment. + +This means that the first deployment of your app must happen manually. + +To do this: + +1. Ensure your code is backwards compatible +1. From the root of this repo run `CF_APP= make cf-push` + +Once this is done, you can push your deployment changes to jenkins to have your app deployed on every deployment. From f712bafc7f6b71087485863472d1459bbad20b0e Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Wed, 15 Sep 2021 17:09:04 +0100 Subject: [PATCH 2/3] Add new guidance on writing public APIs In response to: https://github.com/alphagov/notifications-api/pull/3305#issuecomment-896631475 --- README.md | 1 + docs/writing-public-apis.md | 50 +++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 docs/writing-public-apis.md diff --git a/README.md b/README.md index 064ef1023..cea203f5c 100644 --- a/README.md +++ b/README.md @@ -121,3 +121,4 @@ All commands and command options have a --help command if you need more informat ## Further documentation - [Creating a new worker app](docs/creating-a-new-worker-app.md) +- [Writing public APIs](docs/writing-public-apis.md) diff --git a/docs/writing-public-apis.md b/docs/writing-public-apis.md new file mode 100644 index 000000000..3bca46683 --- /dev/null +++ b/docs/writing-public-apis.md @@ -0,0 +1,50 @@ +# Writing public APIs + +_Most of the API endpoints in this repo are for internal use. These are all defined within top-level folders under `app/` and tend to have the structure `app//rest.py`._ + +## Overview + +Public APIs are intended for use by services and are all located under `app/v2/` to distinguish them from internal endpoints. Originally we did have a "v1" public API, where we tried to reuse / expose existing internal endpoints. The needs for public APIs are sufficiently different that we decided to separate them out. Any "v1" endpoints that remain are now purely internal and no longer exposed to services. + +## New APIs + +Here are some pointers for how we write public API endpoints. + +### Each endpoint should be in its own file in a feature folder + +Example: `app/v2/inbound_sms/get_inbound_sms.py` + +This helps keep the file size manageable but does mean a bit more work to register each endpoint if we have many that are related. Note that internal endpoints are grouped differently: in large `rest.py` files. + +### Each group of endpoints should have an `__init__.py` file + +Example: + +``` +from flask import Blueprint + +from app.v2.errors import register_errors + +v2_notification_blueprint = Blueprint("v2_notifications", __name__, url_prefix='/v2/notifications') + +register_errors(v2_notification_blueprint) +``` + +Note that the error handling setup by `register_errors` (defined in [`app/v2/errors.py`](../app/v2/errors.py)) for public API endpoints is different to that for internal endpoints (defined in [`app/errors.py`](../app/errors.py)). + +### Each endpoint should have an adapter in each API client + +Example: [Ruby Client adapter to get template by ID](https://github.com/alphagov/notifications-ruby-client/blob/d82c85452753b97e8f0d0308c2262023d75d0412/lib/notifications/client.rb#L110-L115). + +All our clients should fully support all of our public APIs. + +Each adapter should be documented in each client ([example](https://github.com/alphagov/notifications-ruby-client/blob/d82c85452753b97e8f0d0308c2262023d75d0412/DOCUMENTATION.md#get-a-template-by-id)). We should also document each public API endpoint in our generic API docs ([example](https://github.com/alphagov/notifications-tech-docs/blob/2700f1164f9d644c87e4c72ad7223952288e8a83/source/documentation/_api_docs.md#send-a-text-message)). Note that internal endpoints are not documented anywhere. + +### Each endpoint should specify the authentication it requires + +This is done as part of registering the blueprint in `app/__init__.py` e.g. + +``` +post_broadcast.before_request(requires_auth) +application.register_blueprint(post_broadcast) +``` From d1826af3c7f0918e2728615ba758ad4d2cc8a39a Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Fri, 17 Sep 2021 16:53:42 +0100 Subject: [PATCH 3/3] Remove out-of-date docs on new workers In response to: https://github.com/alphagov/notifications-api/pull/3332#issuecomment-921897641 These were so out-of-date as to be misleading. We can always refer back to them in the version history if needed. --- README.md | 1 - docs/creating-a-new-worker-app.md | 24 ------------------------ 2 files changed, 25 deletions(-) delete mode 100644 docs/creating-a-new-worker-app.md diff --git a/README.md b/README.md index cea203f5c..e463ac5ee 100644 --- a/README.md +++ b/README.md @@ -120,5 +120,4 @@ All commands and command options have a --help command if you need more informat ## Further documentation -- [Creating a new worker app](docs/creating-a-new-worker-app.md) - [Writing public APIs](docs/writing-public-apis.md) diff --git a/docs/creating-a-new-worker-app.md b/docs/creating-a-new-worker-app.md deleted file mode 100644 index 958594578..000000000 --- a/docs/creating-a-new-worker-app.md +++ /dev/null @@ -1,24 +0,0 @@ -# To create a new worker app - -You need to: -1. Create new entries for your app in `manifest.yml.j2` and `scripts/paas_app_wrapper.sh` ([example](https://github.com/alphagov/notifications-api/pull/2486/commits/6163ca8b45813ff59b3a879f9cfcb28e55863e16)) -1. Update the jenkins deployment job in the notifications-aws repo ([example](https://github.com/alphagov/notifications-aws/commit/69cf9912bd638bce088d4845e4b0a3b11a2cb74c#diff-17e034fe6186f2717b77ba277e0a5828)) -1. Add the new worker's log group to the list of logs groups we get alerts about and we ship them to kibana ([example](https://github.com/alphagov/notifications-aws/commit/69cf9912bd638bce088d4845e4b0a3b11a2cb74c#diff-501ffa3502adce988e810875af546b97)) -1. Optionally add it to the autoscaler ([example](https://github.com/alphagov/notifications-paas-autoscaler/commit/16d4cd0bdc851da2fab9fad1c9130eb94acf3d15)) - -**Important:** - -Before pushing the deployment change on jenkins, read below about the first time deployment. - -## First time deployment of your new worker - -Our deployment flow requires that the app is present in order to proceed with the deployment. - -This means that the first deployment of your app must happen manually. - -To do this: - -1. Ensure your code is backwards compatible -1. From the root of this repo run `CF_APP= make cf-push` - -Once this is done, you can push your deployment changes to jenkins to have your app deployed on every deployment.