ulsdevteam / ccvgd-integral

Docker based deployment of the CCVGD web interface
2 stars 2 forks source link

Production flag should trigger production configuration of frontend Angular #9

Open ctgraham opened 1 year ago

ctgraham commented 1 year ago

The PRODUCTION_FLAG variable in the .env file should trigger ccvgd-frontend to configure the container as production, either by swapping the environment.prod.ts file for the environment.ts file, or rebuilding with ng build --prod.

As is, ccvgd-frontend is always running in dev mode, as reported by the web browser console:

Angular is running in development mode. Call enableProdMode() to enable production mode.
Caligula1022 commented 1 year ago

I add the --prod in the dockerfile and package.json both. You can check now.

ctgraham commented 1 year ago

I think this will always run the frontend container in production mode. Is there any benefit to having the container switch from dev to production mode based on the PRODUCTION_FLAG for development purposes? Or are you happy with the current configuration?

I do see a fair amount of noise in the console which looks like leftover debug code in the Angular application, even when built as prod:

post 
Object { villageid: (1) […], topic: (13) […] }
main-es2015.ce41872983a0508184cf.js:1:791258
post 
Object { villageid: (2) […], topic: (13) […] }
main-es2015.ce41872983a0508184cf.js:1:791258
[debug] before send 
Array [ {…}, {…} ]
main-es2015.ce41872983a0508184cf.js:1:810213
post 
Object { villageid: (2) […], topic: (1) […] }
main-es2015.ce41872983a0508184cf.js:1:791258
[debug] url: https://x.chinesevillagedata.library.pitt.edu https://x.chinesevillagedata.library.pitt.edu/multi-village-search-result main-es2015.ce41872983a0508184cf.js:1:812073
post 
Object { villageid: (3) […], topic: (13) […] }
main-es2015.ce41872983a0508184cf.js:1:791258
post 
Object { villageid: (4) […], topic: (13) […] }
main-es2015.ce41872983a0508184cf.js:1:791258
this response 
Array(13) [ {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, … ]
main-es2015.ce41872983a0508184cf.js:1:808309
this response 
Array(13) [ {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, … ]
main-es2015.ce41872983a0508184cf.js:1:808309

https://github.com/ulsdevteam/ccvgd-frontend/search?q=console.log

Is it worth making these console.log() statements conditional on ! environment.production?

Caligula1022 commented 1 year ago

It's not quite clear why the production_flag doesn't work in the docker compose file. Maybe we can create two dockerfiles, one for dev and one for prod.

I'm not sure what these debugging statements mean in particular. That might be something to ask our frontend developer, Peilin. My personal guess is that these debugging statements are of little use and can be commented. By the way, there are some new features on the frontend that need to be updated by Peilin as well.

Caligula1022 commented 1 year ago

Are there any other issues except this one?

ctgraham commented 1 year ago

In the ccvgd-integral docker-compose.yml, the environment variable PRODUCTION_FLAG is exposed to the running application via the sharing of the .env file: https://github.com/ulsdevteam/ccvgd-integral/blob/a96d90d1745e2340706a76f675af7cb63d6e747d/docker-compose.yml#L49 Note that this should make the assignment of API_URL in the docker-compose.yml unneeded: https://github.com/ulsdevteam/ccvgd-integral/blob/a96d90d1745e2340706a76f675af7cb63d6e747d/docker-compose.yml#L50

During the build process, however, the PRODUCTION_FLAG is passed to the Dockerfile as the PRODUCTION variable: https://github.com/ulsdevteam/ccvgd-integral/blob/a96d90d1745e2340706a76f675af7cb63d6e747d/docker-compose.yml#L44 The build process for the Dockerfile of ccvgd-frontend will not otherwise inherit variables from the ccvg-integral .env file.

In the Dockerfile for ccvgd-frontend and the docker-compose.yml for ccvgd-integral you could use ARG and args parameters, respectively, to pass args from docker compose which could be consumed as an ARG in the Dockerfile.

I have requested a security scan of the application; once that is complete I will open public access to 80/443 and you can review if the ULS-hosted version matches your expectations.

Caligula1022 commented 1 year ago

Thank you so much!