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

MAP-1665 Migrate to the new Alerts API #2363

Merged
merged 11 commits into from
Oct 24, 2024
Merged

MAP-1665 Migrate to the new Alerts API #2363

merged 11 commits into from
Oct 24, 2024

Conversation

tobyprivett
Copy link
Contributor

@tobyprivett tobyprivett commented Oct 11, 2024

Jira link

MAP-1665

What?

  • Replaced the Prison API (Alerts / Alert Codes / Alert Types)
    with the Alerts API (Alerts / Alert Types)
  • Renamed the oauth client NomisClient to HmppsClient

Why?

The Prison API Alerts endpoint has been deprecated.

Deployment risks

@tobyprivett tobyprivett force-pushed the MAP-1665-alerts-api branch 4 times, most recently from 8c012c8 to 57f968e Compare October 18, 2024 13:11
@tobyprivett tobyprivett changed the title MAP-1665 alerts api MAP-1665 Migrate to the new Alerts API Oct 18, 2024
@tobyprivett tobyprivett force-pushed the MAP-1665-alerts-api branch 2 times, most recently from 1f7317e to 3845398 Compare October 21, 2024 09:20
@tobyprivett tobyprivett marked this pull request as ready for review October 22, 2024 08:19
@tobyprivett tobyprivett requested a review from a team as a code owner October 22, 2024 08:19
Copy link
Contributor

@Mjwillis Mjwillis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments ... but LGTM

"ALERTS_API_BASE_URL": {
"description": "Base URL for Alerts API",
"required": true
},
"NOMIS_SITE_FOR_API": {
"description": "Base URL for NOMIS API",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should call this prison-api

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both Prison API and NOMIS API are used all over the app, unfortunately.

@@ -15,6 +15,10 @@
"description": "Name of the storage bucket to use on AWS S3",
"required": true
},
"ALERTS_API_BASE_URL": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all URL added to config?

@@ -41,6 +41,7 @@ generic-service:
NOMIS_API_PATH_PREFIX: nomis_api_path_prefix_key
NOMIS_PRISON_API_PATH_PREFIX: nomis_prison_api_path_prefix_key
NOMIS_AUTH_PATH_PREFIX: nomis_auth_path_prefix_key
ALERTS_API_BASE_URL: alerts_api_base_url_key
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes!

@@ -36,6 +36,7 @@ generic-service:
NOMIS_AUTH_SCHEME: nomis_auth_scheme_key
NOMIS_API_PATH_PREFIX: nomis_api_path_prefix_key
NOMIS_PRISON_API_PATH_PREFIX: nomis_prison_api_path_prefix_key
ALERTS_API_BASE_URL: alerts_api_base_url_key
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do the values get addded?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Kube secrets

@tobyprivett tobyprivett merged commit 7045f7e into main Oct 24, 2024
8 checks passed
@tobyprivett tobyprivett deleted the MAP-1665-alerts-api branch October 24, 2024 09:58
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