Collective lift and shift updates by jonrandahl · Pull Request #188 · epimorphics/ppd-explorer

jonrandahl · 2023-01-24T14:58:39Z

This PR includes the following updates and resolves the linked issues:

Refactors the elapsed time calculated for API requests to be resolved as microseconds rather than milliseconds. This is to improve the reporting of the elapsed time in the system tooling logs. #104/#111
Minor text changes to the Gemfile to include instructions for running Epimorphics specific gems locally during the development of those gems.
Updated the production data_services_api gem version to be at least the current version~>1.3.2 (this is to cover out of sync release versions)
Updated the production lr_common_styles gem version to be at least the current version ~>1.9.0 (this is to cover out of sync release versions)
Refactored better guards in entrypoint.sh to ensure the required env vars are set accordingly or deployment will fail noisily.
Refactored the error messages displayed to be semantically formatted as well as incorporated ALL lines for the returned message to be held in the variable instead of being possibly displayed outside of the intended context.
Adjusted the processing of error status to use the application template; also includes adjustments to not double render the error response. #103
Added the updated configuration for the AWS credential improvements as per other readme's; included further info to run the application locally; resolved a markdown linting issue with using HTML in markdown.

Refactored the version cadence creation to include a SUFFIX value if provided; otherwise no SUFFIX is included in the version number.

Expands Method block length rule to allow for slightly longer method names

Following community guidance for measuring elapsed time in rails the logging mechanism has been changed to use the CPU time instead of RealTime. This also has the added benefit to return the results in the requested `microseconds` unit without further mathematical manipulation.

Refined text changes to the `Gemfile` to include instructions for running Epimorphics specific gems locally during the development of those gems. Updated the production environment gem versions to be at least the current version to cover out of sync release versions. Also includes slight reordering of specific groups of included gems for different environments.

This sets the cache control headers for HMLR apps to be public and cacheable in the production environment. Price Paid Data uses a time limit of 5 minutes (300 seconds) epimorphics/hmlr-linked-data#114

In Production no default URL values are passed on the basis that missing configuration options represent a category of bug, and in that case the deployment should fail fast and noisily. This does include a default value of `true` for the `RAILS_SERVE_STATIC_FILES` nonetheless. https://github.com/epimorphics/ijd-transfer-notes/blob/main/rails-app-pattern.md#environment-variables-and-run-time-configuration

In order to improve the ux minor tweaks have been made to the layout and formatting of the returned error message and reference code presentation.

Following lengthy internal discussions the global env variable `RAILS_RELATIVE_URL_ROOT` has been renamed to `APPLICATION_PATH` to be more clear on it's use; needs to be monitored once deployed. Also includes better guards in `entrypoint.sh` to ensure the required env vars are set accordingly or deployment will fail noisily.

Refactored the `StandardError` rescue block to use the provided status from the current exception outside of `MalformedSearchError` and `ArgumentError` types and otherwise fall back to a generic 500 status;

Refactored the error messages displayed to be semantically formatted as well as incorporated ALL lines for the returned message to be held in the variable instead of being possibly displayed outside of the intended context.

Also includes minor update to layout format for older entries

…epimorphics/ppd-explorer into spike/collective-lift-and-shift-updates

Adjusted the processing of error status to use the application template; also includes adjustments to not double render the error response. epimorphics/hmlr-linked-data#103

…updates' into spike/collective-lift-and-shift-updates

refactored the gem paths to use the remote Epimorphics repository locations, also includes the regenerated gemfile.lock reflecting as such.

Added the updated configuration for the AWS credential improvements as per other readme's; included further info to run the application locally; resolved a markdown linting issue with using HTML in markdown.

This resolves that Rails would want the `RAILS_RELATIVE_URL_ROOT` env var set, while using `APPLICATION_ROOT` is clearer in the `entrypoint.sh` code - this also updates the readme’s to reflect this change

Fixed typo for `APPLICATION_ROOT` env var

…t-updates

fixes variable for development environments

…updates' into spike/collective-lift-and-shift-updates

bogdanadrianmarc

Looks good 👍

der

Looks fine, just one comment in line.

Dockerfile

Implements unification approach to Make, Docker and other associated build files to ensure the image process completes as expected.

as these are not used in the app they were actually causing more issue on production images and therefore were removed.

now mirrors other app structures

`config.assets.precompile` and `config.assets.version` have moved to `config/initializers/assets.rb`

Updated the matching configuration settings on each environment configuration file to uniify the settings for all apps

andrew-pickin-epi · 2023-03-21T17:36:07Z

Makefile

 COMMIT=$(shell git rev-parse --short HEAD)
 VERSION?=$(shell /usr/bin/env ruby -e 'require "./app/lib/version" ; puts Version::VERSION')
-TAG?=$(shell printf '%s-%s-%08d' ${VERSION} ${COMMIT} ${GITHUB_RUN_NUMBER})
+TAG?=$(shell printf '%s_%s_%08d' ${VERSION} ${COMMIT} ${GITHUB_RUN_NUMBER})


Has it been confirmed that changing the Tag format doesn't break deployment?

andrew-pickin-epi · 2023-03-21T17:38:32Z

README.md

 | name                       | description                                                          | typical value              |
 | -------------------------- | -------------------------------------------------------------------- | -------------------------- |
-| `RAILS_RELATIVE_URL_ROOT`  | The path from the server root to the application                     | `/app/ppd`                 |
+| `APPLICATION_ROOT`         | The path from the server root to the application                     | `/app/ppd`                 |


NO. This shouldn't be an environment variable unless the image has no knowledge of the path from build time. I VERY much suspect this is not the case.

What does "Typical Value" mean?

andrew-pickin-epi · 2023-03-21T17:40:48Z

entrypoint.sh

 fi

-export RAILS_RELATIVE_URL_ROOT=${RAILS_RELATIVE_URL_ROOT:-'/app/ppd'}
+export RAILS_RELATIVE_URL_ROOT=${APPLICATION_ROOT:-'/app/ppd'}


No. Don't use environment variables.

andrew-pickin-epi · 2023-03-21T17:41:17Z

entrypoint.sh

 export SCRIPT_NAME=${RAILS_RELATIVE_URL_ROOT}

-echo "{'ts': '`date -u +%FT%T.%3NZ`', 'message': {'text: 'Starting Standard Reports UI. API_SERVICE_URL=${API_SERVICE_URL} RAILS_ENV=${RAILS_ENV} RAILS_RELATIVE_URL_ROOT=${RAILS_RELATIVE_URL_ROOT}', 'level': 'INFO'}}"
+echo "{\"ts\": $(date -u +%FT%T.%3NZ), \"level\": \"INFO\", \"message\": \"Starting ${PUBLIC_NAME} application with RAILS_ENV=\"${RAILS_ENV}\", \"RAILS_RELATIVE_URL_ROOT\"=\"${RAILS_RELATIVE_URL_ROOT}\", \"SCRIPT_NAME\"=\"${SCRIPT_NAME}\"}"


No. Don't use environment variables.

der

Looks fine to me apart from not understanding the point of SCRIPT_NAME.

Don't understand @andrew-pickin-epi 's concern about use of environment variable to set RAILS_RELATIVE_URL_ROOT but once that's resolved to mutual satisfaction I don't think I need to re-review so approving now.

der · 2023-03-21T19:04:13Z

entrypoint.sh


-export RAILS_RELATIVE_URL_ROOT=${RAILS_RELATIVE_URL_ROOT:-'/app/ppd'}
+export RAILS_RELATIVE_URL_ROOT=${APPLICATION_ROOT:-'/app/ppd'}
 export SCRIPT_NAME=${RAILS_RELATIVE_URL_ROOT}


Is this needed? I thought we only used RAILS_RELATIVE_URL_ROOT

…ollective-lift-and-shift-updates

improvements for handling error pages. will need to be ported onto the other apps as well post lift & shift testing

as per other HMLR apps

specific to epimorphics internal gems

andrew-pickin-epi

I suspect there a broken merge going on here.
The Makefile is certainly in error.

andrew-pickin-epi · 2023-05-15T15:42:55Z

CHANGELOG.md

+- (Jon) Refactored the version cadence creation to include a SUFFIX value if
+  provided; otherwise no SUFFIX is included in the version number.
+- (Jon) Adjusted the processing of error status to use the application template;
+  also includes adjustments to not double render the error response.


APPLICATION_ROOT has been dropped.

andrew-pickin-epi · 2023-05-15T15:43:44Z

README.md

+
+| name                       | description                                                          | typical value              |
+| -------------------------- | -------------------------------------------------------------------- | -------------------------- |
+| `APPLICATION_ROOT`         | The path from the server root to the application                     | `/app/ppd`                 |


APPLICATION_ROOT has been dropped.

andrew-pickin-epi · 2023-05-15T15:46:07Z

Makefile

 COMMIT=$(shell git rev-parse --short HEAD)
 VERSION?=$(shell /usr/bin/env ruby -e 'require "./app/lib/version" ; puts Version::VERSION')
-TAG?=$(shell printf '%s-%s-%08d' ${VERSION} ${COMMIT} ${GITHUB_RUN_NUMBER})
+TAG?=$(shell printf '%s_%s_%08d' ${VERSION} ${COMMIT} ${GITHUB_RUN_NUMBER})


This still using the wrong path for the bundle conf.
Broken merge?

Removed `${HOME}/` from path

Includes further clarification on running the app individually, or as part of the HMLR suite of apps; alongside using Docker containers and accessing the Data API locally.

jonrandahl added 12 commits January 24, 2023 13:20

build: updated version cadence

16cf719

Refactored the version cadence creation to include a SUFFIX value if provided; otherwise no SUFFIX is included in the version number.

styles: Updated rubocop settings

ee7e39c

Expands Method block length rule to allow for slightly longer method names

docs: Updated CHANGELOG

5402da9

feat: static asset cache headers

e41b0b1

This sets the cache control headers for HMLR apps to be public and cacheable in the production environment. Price Paid Data uses a time limit of 5 minutes (300 seconds) epimorphics/hmlr-linked-data#114

styles: Better error page formatting

c01ac75

In order to improve the ux minor tweaks have been made to the layout and formatting of the returned error message and reference code presentation.

fix: better error control

ae81584

Refactored the `StandardError` rescue block to use the provided status from the current exception outside of `MalformedSearchError` and `ArgumentError` types and otherwise fall back to a generic 500 status;

fix: better error message display

dca3d74

Refactored the error messages displayed to be semantically formatted as well as incorporated ALL lines for the returned message to be held in the variable instead of being possibly displayed outside of the intended context.

docs: Updated CHANGELOG

68f3700

Also includes minor update to layout format for older entries

jonrandahl self-assigned this Jan 26, 2023

jonrandahl and others added 10 commits January 27, 2023 09:29

docs: Updated CHANGELOG

9b3fb9b

Docker build optimizations

420638f

Merge branch 'spike/collective-lift-and-shift-updates' of github.com:…

37a1501

…epimorphics/ppd-explorer into spike/collective-lift-and-shift-updates

fix: Resolved Missing Template for errors

bd0e195

Adjusted the processing of error status to use the application template; also includes adjustments to not double render the error response. epimorphics/hmlr-linked-data#103

Merge remote-tracking branch 'origin/spike/collective-lift-and-shift-…

840f5a5

…updates' into spike/collective-lift-and-shift-updates

docs: Updated CHANGELOG

5f0117b

ci: resolving gem locations

56e32b0

refactored the gem paths to use the remote Epimorphics repository locations, also includes the regenerated gemfile.lock reflecting as such.

docs: Updated README

686dced

Added the updated configuration for the AWS credential improvements as per other readme's; included further info to run the application locally; resolved a markdown linting issue with using HTML in markdown.

docs: Updated CHANGELOG

c654df0

docs: Updated CHANGELOG

af8d910

jonrandahl marked this pull request as ready for review March 10, 2023 17:50

jonrandahl and others added 3 commits March 13, 2023 11:24

fix: resolve RAILS_RELATIVE_URL_ROOT env var

9a14c61

This resolves that Rails would want the `RAILS_RELATIVE_URL_ROOT` env var set, while using `APPLICATION_ROOT` is clearer in the `entrypoint.sh` code - this also updates the readme’s to reflect this change

docs: Updated CHANGELOG

a97cab7

Fixed typo for `APPLICATION_ROOT` env var

Merge branch 'dev-infrastructure' into spike/collective-lift-and-shif…

2f1428c

…t-updates

jonrandahl requested review from bogdanadrianmarc and der March 13, 2023 11:55

fix: resolve RAILS_RELATIVE_URL_ROOT env var

afa7edf

fixes variable for development environments

jonrandahl added 2 commits March 13, 2023 12:06

Merge remote-tracking branch 'origin/spike/collective-lift-and-shift-…

f3d75f6

…updates' into spike/collective-lift-and-shift-updates

docs: Updated README

2566f23

jonrandahl changed the title ~~Spike/collective lift and shift updates~~ Collective lift and shift updates Mar 13, 2023

bogdanadrianmarc approved these changes Mar 13, 2023

View reviewed changes

der approved these changes Mar 13, 2023

View reviewed changes

Dockerfile Show resolved Hide resolved

jonrandahl added 5 commits March 20, 2023 22:31

build: cleanup of build files for LR apps

6dd5fbe

Implements unification approach to Make, Docker and other associated build files to ensure the image process completes as expected.

fix: removed unused rescues

b4ea46c

as these are not used in the app they were actually causing more issue on production images and therefore were removed.

refactor: rearranging code blocks

a7af53d

now mirrors other app structures

refactor: mirrored config setting across all apps

ea341a5

`config.assets.precompile` and `config.assets.version` have moved to `config/initializers/assets.rb`

build: unified environment configurations across all apps

e298f84

Updated the matching configuration settings on each environment configuration file to uniify the settings for all apps

jonrandahl requested review from andrew-pickin-epi and der March 21, 2023 13:06

andrew-pickin-epi requested changes Mar 21, 2023

View reviewed changes

der approved these changes Mar 21, 2023

View reviewed changes

jonrandahl added 6 commits April 19, 2023 14:51

Merge remote-tracking branch 'origin/dev-infrastructure' into spike/c…

cadc78f

…ollective-lift-and-shift-updates

refactor: error handling now uses templates

97762d9

improvements for handling error pages. will need to be ported onto the other apps as well post lift & shift testing

build: improvements to environment configs

845ba8f

build: streamlined entrypoint script

9b2e52f

as per other HMLR apps

build: gemfile and lock updates

1cd522e

specific to epimorphics internal gems

build: additional environment assets compilation

4a4bae6

jonrandahl requested a review from andrew-pickin-epi May 11, 2023 10:03

andrew-pickin-epi requested changes May 15, 2023

View reviewed changes

jonrandahl added 4 commits May 15, 2023 18:09

build: Cleaned BUNDLE_CONFIG

3ac1c68

Removed `${HOME}/` from path

docs: Updated CHANGELOG

3d9d5a5

docs: Rewrote README

d06447c

Includes further clarification on running the app individually, or as part of the HMLR suite of apps; alongside using Docker containers and accessing the Data API locally.

revert: rolled back tag separator rubocop suggestion

410841e

jonrandahl requested a review from andrew-pickin-epi May 18, 2023 09:37

andrew-pickin-epi approved these changes May 22, 2023

View reviewed changes

jonrandahl merged commit a54f447 into dev-infrastructure May 23, 2023

jonrandahl deleted the spike/collective-lift-and-shift-updates branch May 23, 2023 08:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Collective lift and shift updates#188

Collective lift and shift updates#188
jonrandahl merged 43 commits intodev-infrastructurefrom
spike/collective-lift-and-shift-updates

jonrandahl commented Jan 24, 2023 •

edited

Loading

Uh oh!

bogdanadrianmarc left a comment

Uh oh!

der left a comment

Uh oh!

Uh oh!

andrew-pickin-epi Mar 21, 2023

Uh oh!

andrew-pickin-epi Mar 21, 2023

Uh oh!

andrew-pickin-epi Mar 21, 2023

Uh oh!

andrew-pickin-epi Mar 21, 2023

Uh oh!

der left a comment

Uh oh!

der Mar 21, 2023

Uh oh!

andrew-pickin-epi left a comment

Uh oh!

andrew-pickin-epi May 15, 2023

Uh oh!

andrew-pickin-epi May 15, 2023

Uh oh!

andrew-pickin-epi May 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jonrandahl commented Jan 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bogdanadrianmarc left a comment

Choose a reason for hiding this comment

Uh oh!

der left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

der left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrew-pickin-epi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jonrandahl commented Jan 24, 2023 •

edited

Loading